Hi Rob,
<snip>
> +
> +struct ocmem_config {
> + uint8_t num_regions;
u8
> + uint32_t macro_size;
u32
> +static const struct ocmem_config ocmem_8974_config = {
> + .num_regions = 3, .macro_size = SZ_128K,
> +};
> +
> +static const struct of_device_id ocmem_dt_match[] = {
> + { .compatible = "qcom,ocmem-msm8974", .data = &ocmem_8974_config },
Just a suggestion. As we have REG_OCMEM_HW_PROFILE and
REG_OCMEM_HW_VERSION register why not distinguish by version and hw
profile instead of SoC?
> + {}
> +};
> +
> +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");
> + if (IS_ERR(ocmem->core_clk)) {
> + dev_err(dev, "Unable to get the core clock\n");
this dev_err will flood console in EPROBE_DEFER case, IMO should be removed.
> + return PTR_ERR(ocmem->core_clk);
> + }
> +
> + ocmem->iface_clk = devm_clk_get(dev, "iface_clk");
> + if (IS_ERR_OR_NULL(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;
Why error path of the iface clock is different comparing with core clk.
Also I failed to found a case when common clk framework will return NULL?
if (IS_ERR(ocmem->iface_clk)
return PTR_ERR(ocmem->iface_clk);
this should be enough.
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "ocmem_ctrl_physical");
> + if (!res) {
> + dev_err(&pdev->dev, "failed to get memory resource\n");
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> + ocmem->mmio = devm_ioremap_nocache(&pdev->dev, res->start,
> + resource_size(res));
I think you could use following to simplify ioremap:
res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
"ocmem_ctrl_physical");
ocmem->mmio = devm_ioremap_resource(dev, res);
if (IS_ERR(ocmem->mmio))
return PTR_ERR(ocmem->mmio);
> + if (IS_ERR(ocmem->mmio)) {
> + dev_err(&pdev->dev, "failed to ioremap memory resource\n");
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> + ocmem->ocmem_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + "ocmem_physical");
> + if (!ocmem->ocmem_mem) {
> + dev_err(dev, "could not get OCMEM region\n");
> + return -ENXIO;
> + }
> +
<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
