Hi Kieran, On Fri, Aug 16, 2019 at 09:07:14AM +0100, Kieran Bingham wrote: > On 14/08/2019 15:54, Laurent Pinchart wrote: > > This helps identifying the IP core version, for debugging purpose only > > for now. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com> > > --- > > Changes since v1: > > > > - Use devm_platform_ioremap_resource() > > --- > > drivers/media/platform/rcar-fcp.c | 41 +++++++++++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/drivers/media/platform/rcar-fcp.c > > b/drivers/media/platform/rcar-fcp.c > > index 43c78620c9d8..6e0c0e7c0f8c 100644 > > --- a/drivers/media/platform/rcar-fcp.c > > +++ b/drivers/media/platform/rcar-fcp.c > > @@ -8,6 +8,7 @@ > > */ > > > > #include <linux/device.h> > > +#include <linux/io.h> > > #include <linux/list.h> > > #include <linux/module.h> > > #include <linux/mod_devicetable.h> > > @@ -21,11 +22,38 @@ > > struct rcar_fcp_device { > > struct list_head list; > > struct device *dev; > > + void __iomem *iomem; > > }; > > > > static LIST_HEAD(fcp_devices); > > static DEFINE_MUTEX(fcp_lock); > > > > +#define FCP_VCR 0x0000 > > +#define FCP_VCR_CATEGORY_MASK (0xff << 8) > > +#define FCP_VCR_CATEGORY_SHIFT 8 > > +#define FCP_VCR_REVISION_MASK (0xff << 0) > > +#define FCP_VCR_REVISION_SHIFT 0 > > + > > +#define FCP_CFG0 0x0004 > > +#define FCP_RST 0x0010 > > +#define FCP_STA 0x0018 > > +#define FCP_TL_CTRL 0x0070 > > +#define FCP_PICINFO1 0x00c4 > > +#define FCP_BA_ANC_Y0 0x0100 > > +#define FCP_BA_ANC_Y1 0x0104 > > +#define FCP_BA_ANC_Y2 0x0108 > > +#define FCP_BA_ANC_C 0x010c > > +#define FCP_BA_REF_Y0 0x0110 > > +#define FCP_BA_REF_Y1 0x0114 > > +#define FCP_BA_REF_Y2 0x0118 > > +#define FCP_BA_REF_C 0x011c > > Do we need to pull in all these extra register definitions just to read > the version? > > They don't hurt if they're for something else later...
At least FCP_CFG0 will be used for FCNL. Some of the other registers could be left out, but I don't think they really hurt either, they don't take any space in the compiled object. > Otherwise, > > Reviewed-by: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com> > > > + > > + > > +static inline u32 rcar_fcp_read(struct rcar_fcp_device *fcp, u32 reg) > > +{ > > + return ioread32(fcp->iomem + reg); > > +} > > + > > /* > > ----------------------------------------------------------------------------- > > * Public API > > */ > > @@ -129,6 +157,7 @@ EXPORT_SYMBOL_GPL(rcar_fcp_disable); > > static int rcar_fcp_probe(struct platform_device *pdev) > > { > > struct rcar_fcp_device *fcp; > > + u32 version; > > > > fcp = devm_kzalloc(&pdev->dev, sizeof(*fcp), GFP_KERNEL); > > if (fcp == NULL) > > @@ -138,6 +167,18 @@ static int rcar_fcp_probe(struct platform_device *pdev) > > > > pm_runtime_enable(&pdev->dev); > > > > + fcp->iomem = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(fcp->iomem)) > > + return PTR_ERR(fcp->iomem); > > + > > + pm_runtime_get_sync(&pdev->dev); > > + version = rcar_fcp_read(fcp, FCP_VCR); > > + pm_runtime_put(&pdev->dev); > > + > > + dev_dbg(&pdev->dev, "FCP category %u revision %u\n", > > + (version & FCP_VCR_CATEGORY_MASK) >> FCP_VCR_CATEGORY_SHIFT, > > + (version & FCP_VCR_REVISION_MASK) >> FCP_VCR_REVISION_SHIFT); > > + > > mutex_lock(&fcp_lock); > > list_add_tail(&fcp->list, &fcp_devices); > > mutex_unlock(&fcp_lock); -- Regards, Laurent Pinchart