On Fri, Oct 23, 2015 at 8:11 AM, Stanimir Varbanov <svarbanov@xxxxxxxxxx> wrote: > Hi Rob, > > On 10/13/2015 12:45 AM, Rob Clark wrote: >> The OCMEM driver handles allocation and configuration of the On Chip >> MEMory that is present on some snapdragon devices. >> >> Devices which have OCMEM do not have GMEM inside the gpu core, so the >> gpu must instead use OCMEM to be functional. Since currently the gpu >> is the only OCMEM user with an upstream driver, this is just a minimal >> implementation sufficient for statically allocating to the gpu it's >> chunk of OCMEM. >> >> v2: minor updates from review comments (use devm_ioremap_resource(), >> devm_clk_get() doesn't return NULL, etc..) >> >> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx> >> --- >> drivers/soc/qcom/Kconfig | 6 + >> drivers/soc/qcom/Makefile | 1 + >> drivers/soc/qcom/ocmem.c | 401 +++++++++++++++++++++++++++++++++++++++++++ >> drivers/soc/qcom/ocmem.xml.h | 106 ++++++++++++ >> include/soc/qcom/ocmem.h | 40 +++++ >> 5 files changed, 554 insertions(+) >> create mode 100644 drivers/soc/qcom/ocmem.c >> create mode 100644 drivers/soc/qcom/ocmem.xml.h >> create mode 100644 include/soc/qcom/ocmem.h >> >> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig >> index 266060b..b485043 100644 >> --- a/drivers/soc/qcom/Kconfig >> +++ b/drivers/soc/qcom/Kconfig >> @@ -74,3 +74,9 @@ config QCOM_WCNSS_CTRL >> help >> Client driver for the WCNSS_CTRL SMD channel, used to download nv >> firmware to a newly booted WCNSS chip. >> + >> +config QCOM_OCMEM >> + tristate "Qualcomm OCMEM driver" >> + depends on QCOM_SMD > > Why this driver depends on SMD, I cannot see where smd is used here. > > Maybe depends on ARCH_QCOM should be sufficient? yeah, that should have been "SCM".. the acronyms are too close together ;-) > >> + help >> + Allocator for OCMEM (On Chip MEMory). >> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile >> index fdd664e..940bb35 100644 >> --- a/drivers/soc/qcom/Makefile >> +++ b/drivers/soc/qcom/Makefile >> @@ -7,3 +7,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o >> obj-$(CONFIG_QCOM_SMP2P) += smp2p.o >> obj-$(CONFIG_QCOM_SMSM) += smsm.o >> obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o >> +obj-$(CONFIG_QCOM_OCMEM) += ocmem.o >> diff --git a/drivers/soc/qcom/ocmem.c b/drivers/soc/qcom/ocmem.c >> new file mode 100644 >> index 0000000..506a072 >> --- /dev/null >> +++ b/drivers/soc/qcom/ocmem.c >> @@ -0,0 +1,401 @@ >> +/* >> + * Copyright (C) 2015 Red Hat >> + * Author: Rob Clark <robdclark@xxxxxxxxx> >> + * >> + * This program is free software; you can redistribute it and/or modify it >> + * under the terms of the GNU General Public License version 2 as published by >> + * the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, but WITHOUT >> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or >> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for >> + * more details. >> + * >> + * You should have received a copy of the GNU General Public License along with >> + * this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/io.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> >> +#include <linux/qcom_scm.h> > > Do we need to update the Kconfig to enable/depend on scm driver? > >> +#include <linux/sizes.h> >> +#include <linux/slab.h> >> +#include <linux/types.h> >> + >> +#include <soc/qcom/ocmem.h> >> +#include "ocmem.xml.h" >> + >> +enum region_mode { >> + WIDE_MODE = 0x0, >> + THIN_MODE, >> + MODE_DEFAULT = WIDE_MODE, >> +}; >> + >> +struct ocmem_region { >> + unsigned psgsc_ctrl; > > this is not used > >> + bool interleaved; >> + enum region_mode mode; >> + unsigned int num_macros; >> + enum ocmem_macro_state macro_state[4]; >> + unsigned long macro_size; >> + unsigned long region_size; >> +}; >> + > > <snip> > >> + >> +static const struct of_device_id ocmem_dt_match[] = { >> + { .compatible = "qcom,ocmem-msm8974", .data = &ocmem_8974_config }, >> + {} >> +}; >> + >> +static int ocmem_dev_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + const struct ocmem_config *config = NULL; >> + const struct of_device_id *match; >> + struct resource *res; >> + uint32_t reg, num_banks, region_size; >> + int i, j, ret; >> + >> + /* we need scm to be available: */ >> + if (!qcom_scm_is_available()) >> + return -EPROBE_DEFER; >> + >> + match = of_match_device(ocmem_dt_match, dev); >> + if (match) >> + config = match->data; >> + >> + if (!config) { >> + dev_err(dev, "unknown config: %s\n", dev->of_node->name); >> + return -ENXIO; >> + } >> + >> + ocmem = devm_kzalloc(dev, sizeof(*ocmem), GFP_KERNEL); >> + if (!ocmem) >> + return -ENOMEM; >> + >> + ocmem->dev = dev; >> + ocmem->config = config; >> + >> + ocmem->core_clk = devm_clk_get(dev, "core_clk"); > > We have a convention to avoid _clk suffix for qcom clocks in dt nodes. > >> + if (IS_ERR(ocmem->core_clk)) { >> + dev_info(dev, "Unable to get the core clock\n"); >> + return PTR_ERR(ocmem->core_clk); >> + } >> + >> + ocmem->iface_clk = devm_clk_get(dev, "iface_clk"); >> + if (IS_ERR(ocmem->iface_clk)) { >> + ret = PTR_ERR(ocmem->iface_clk); >> + ocmem->iface_clk = NULL; >> + /* in probe-defer case, propagate error up and try again later: */ >> + if (ret == -EPROBE_DEFER) >> + goto fail; >> + } >> + >> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, >> + "ocmem_ctrl_physical"); >> + ocmem->mmio = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(ocmem->mmio)) { >> + dev_err(&pdev->dev, "failed to ioremap memory resource\n"); >> + ret = -EINVAL; > > ret = PTR_ERR(ocmem->mmio); ? > >> + goto fail; >> + } >> + > > <snip> > >> + >> +static bool ocmem_exists(void) >> +{ >> + struct device_driver *drv = &ocmem_driver.driver; >> + struct device *d; >> + >> + d = bus_find_device(&platform_bus_type, NULL, drv, >> + (void *)platform_bus_type.match); >> + if (d) { >> + put_device(d); >> + return true; >> + } >> + >> + return false; >> +} > > do you have a dt binding document? I think that ocmem dt client nodes > should have a phandle to ocmem dt node and this could eliminate the need > of ocmem_exist(), no ? so far just generic bindings (ie. compatible/clocks/reg) so I didn't write one.. are thinks which don't introduce any custom properties supposed to be documented? Not sure about the phandle link.. I didn't think it influenced probe order (and if another ocmem client needed to access it from it's probe()...) BR, -R > > <snip> > > -- > regards, > Stan -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
