| Message ID | 20190814145417.30670-1-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | [v2] v4l: rcar-fcp: Read IP version register at probe time | expand |
Hi Laurent, 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+renesas@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... Otherwise, Reviewed-by: Kieran Bingham <kieran.bingham+renesas@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); >
Hi Laurent, On Wed, Aug 14, 2019 at 4:55 PM Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> wrote: > This helps identifying the IP core version, for debugging purpose only > for now. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- a/drivers/media/platform/rcar-fcp.c > +++ b/drivers/media/platform/rcar-fcp.c > @@ -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); Unless (dynamic) debugging is enabled, all of the above is done for obtaining a version number that is not used. Can this be improved? > + > + 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); Gr{oetje,eeting}s, Geert
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+renesas@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+renesas@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);
Hi Geert, On Fri, Aug 16, 2019 at 10:21:42AM +0200, Geert Uytterhoeven wrote: > On Wed, Aug 14, 2019 at 4:55 PM Laurent Pinchart wrote: > > This helps identifying the IP core version, for debugging purpose only > > for now. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > > --- a/drivers/media/platform/rcar-fcp.c > > +++ b/drivers/media/platform/rcar-fcp.c > > > @@ -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); > > Unless (dynamic) debugging is enabled, all of the above is done for obtaining > a version number that is not used. > Can this be improved? With FCNL support we'll need that anyway. This patch comes from a larger FCNL series that rejects devices with an unknown version, and I thought it could be fast-tracked in a stripped form already. I don't mind either way, I can wait until it's time to merge FCNL support. > > + > > + 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);
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 + + +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);
This helps identifying the IP core version, for debugging purpose only for now. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- Changes since v1: - Use devm_platform_ioremap_resource() --- drivers/media/platform/rcar-fcp.c | 41 +++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)