On 7/1/19 8:27 AM, Pawel Dembicki wrote: > This patch add platform part of vsc73xx driver. > It allows to use chip connected by PI interface. > > Signed-off-by: Pawel Dembicki <paweldembicki@xxxxxxxxx> > --- [snip] > + struct vsc73xx_platform *vsc_platform = vsc->priv; > + u32 offset; > + > + if (!vsc73xx_is_addr_valid(block, subblock)) > + return -EINVAL; > + > + offset = vsc73xx_make_addr(block, subblock, reg); > + > + mutex_lock(&vsc->lock); > + iowrite32be(val, vsc_platform->base_addr + offset); > + mutex_unlock(&vsc->lock); Similar question from Andrew, why is the locking done in the platform layer, should not that be done in the core I/O operation instead? > + > + return 0; > +} > + > +static int vsc73xx_platform_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct vsc73xx_platform *vsc_platform; > + struct resource *res = NULL; > + int ret; > + > + vsc_platform = devm_kzalloc(dev, sizeof(*vsc_platform), GFP_KERNEL); > + if (!vsc_platform) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, vsc_platform); > + vsc_platform->pdev = pdev; > + vsc_platform->vsc.dev = dev; > + vsc_platform->vsc.priv = vsc_platform; > + vsc_platform->vsc.ops = &vsc73xx_platform_ops; > + > + /* obtain I/O memory space */ > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + dev_err(&pdev->dev, "cannot obtain I/O memory space\n"); > + ret = -ENXIO; > + return ret; > + } > + > + vsc_platform->base_addr = devm_ioremap_resource(&pdev->dev, res); devm_ioremap_resource takes care of checking that the resource pointer is valid, no need to do that here. > + if (!vsc_platform->base_addr) { if (IS_ERR(vsc_platform->base_addr)) -- Florian