From: Michael Tretter <m.tretter@pengutronix.de> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org, Philipp Zabel <p.zabel@pengutronix.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Fabio Estevam <festevam@gmail.com>, kernel@pengutronix.de, linux-imx@nxp.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/8] media: imx-pxp: detect PXP version Date: Fri, 6 Jan 2023 15:01:41 +0100 [thread overview] Message-ID: <20230106140141.GC24101@pengutronix.de> (raw) In-Reply-To: <Y7gT/+2c5G9lT8jM@pendragon.ideasonboard.com> On Fri, 06 Jan 2023 14:28:47 +0200, Laurent Pinchart wrote: > On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote: > > Thank you for the patch. > > > > On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote: > > > Different versions of the Pixel Pipeline have different blocks and their > > > routing may be different. Read the PXP_HW_VERSION register to determine > > > the version of the PXP and print it to the log for debugging purposes. > > > > Is there a specific reason you chose to read the version register > > instead of basing the decision on the compatible string ? > > Reading the rest of the series, you use the compatible strings later, > and never use the hw_version field. I'm tempted to propose dropping this > patch. My first try was to use the version register, but it turned out that the version is the same at least on i.MX6ULL and i.MX7D. I kept it to avoid that others fall into the same trap. > > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > > > --- > > > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++ > > > 1 file changed, 18 insertions(+) > > > > > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c > > > index 689ae5e6ac62..05abe40558b0 100644 > > > --- a/drivers/media/platform/nxp/imx-pxp.c > > > +++ b/drivers/media/platform/nxp/imx-pxp.c > > > @@ -10,6 +10,7 @@ > > > * Pawel Osciak, <pawel@osciak.com> > > > * Marek Szyprowski, <m.szyprowski@samsung.com> > > > */ > > > +#include <linux/bitfield.h> > > > #include <linux/clk.h> > > > #include <linux/delay.h> > > > #include <linux/dma-mapping.h> > > > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info"); > > > #define MEM2MEM_HFLIP (1 << 0) > > > #define MEM2MEM_VFLIP (1 << 1) > > > > > > +#define PXP_VERSION_MAJOR(version) \ > > > + FIELD_GET(BM_PXP_VERSION_MAJOR, version) > > > +#define PXP_VERSION_MINOR(version) \ > > > + FIELD_GET(BM_PXP_VERSION_MINOR, version) > > > + > > > #define dprintk(dev, fmt, arg...) \ > > > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg) > > > > > > @@ -192,6 +198,8 @@ struct pxp_dev { > > > struct clk *clk; > > > void __iomem *mmio; > > > > > > + u32 hw_version; > > > + > > > atomic_t num_inst; > > > struct mutex dev_mutex; > > > spinlock_t irqlock; > > > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev) > > > return 0; > > > } > > > > > > +static u32 pxp_read_version(struct pxp_dev *dev) > > > +{ > > > + return readl(dev->mmio + HW_PXP_VERSION); > > > +} > > > + > > > static int pxp_probe(struct platform_device *pdev) > > > { > > > struct pxp_dev *dev; > > > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev) > > > goto err_clk; > > > } > > > > > > + dev->hw_version = pxp_read_version(dev); > > > + dev_info(&pdev->dev, "PXP Version %d.%d\n", > > > > As the version can't be negative, I'd use %u.%u. > > > > > + PXP_VERSION_MAJOR(dev->hw_version), > > > + PXP_VERSION_MINOR(dev->hw_version)); > > > + > > > > The driver now prints two messages at probe time, it would be nice to > > combine them, or remove the other one. That's a candidate for a future > > patch though. I would reduce the level to dev_debug. Then the version is not always printed, but it can be easily enabled if necessary for the bringup on another platform. Michael > > > > > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev); > > > if (ret) > > > goto err_clk;
WARNING: multiple messages have this Message-ID (diff)
From: Michael Tretter <m.tretter@pengutronix.de> To: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org, Philipp Zabel <p.zabel@pengutronix.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Fabio Estevam <festevam@gmail.com>, kernel@pengutronix.de, linux-imx@nxp.com, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 2/8] media: imx-pxp: detect PXP version Date: Fri, 6 Jan 2023 15:01:41 +0100 [thread overview] Message-ID: <20230106140141.GC24101@pengutronix.de> (raw) In-Reply-To: <Y7gT/+2c5G9lT8jM@pendragon.ideasonboard.com> On Fri, 06 Jan 2023 14:28:47 +0200, Laurent Pinchart wrote: > On Fri, Jan 06, 2023 at 01:47:32PM +0200, Laurent Pinchart wrote: > > Thank you for the patch. > > > > On Thu, Jan 05, 2023 at 02:47:23PM +0100, Michael Tretter wrote: > > > Different versions of the Pixel Pipeline have different blocks and their > > > routing may be different. Read the PXP_HW_VERSION register to determine > > > the version of the PXP and print it to the log for debugging purposes. > > > > Is there a specific reason you chose to read the version register > > instead of basing the decision on the compatible string ? > > Reading the rest of the series, you use the compatible strings later, > and never use the hw_version field. I'm tempted to propose dropping this > patch. My first try was to use the version register, but it turned out that the version is the same at least on i.MX6ULL and i.MX7D. I kept it to avoid that others fall into the same trap. > > > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de> > > > --- > > > drivers/media/platform/nxp/imx-pxp.c | 18 ++++++++++++++++++ > > > 1 file changed, 18 insertions(+) > > > > > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c > > > index 689ae5e6ac62..05abe40558b0 100644 > > > --- a/drivers/media/platform/nxp/imx-pxp.c > > > +++ b/drivers/media/platform/nxp/imx-pxp.c > > > @@ -10,6 +10,7 @@ > > > * Pawel Osciak, <pawel@osciak.com> > > > * Marek Szyprowski, <m.szyprowski@samsung.com> > > > */ > > > +#include <linux/bitfield.h> > > > #include <linux/clk.h> > > > #include <linux/delay.h> > > > #include <linux/dma-mapping.h> > > > @@ -52,6 +53,11 @@ MODULE_PARM_DESC(debug, "activates debug info"); > > > #define MEM2MEM_HFLIP (1 << 0) > > > #define MEM2MEM_VFLIP (1 << 1) > > > > > > +#define PXP_VERSION_MAJOR(version) \ > > > + FIELD_GET(BM_PXP_VERSION_MAJOR, version) > > > +#define PXP_VERSION_MINOR(version) \ > > > + FIELD_GET(BM_PXP_VERSION_MINOR, version) > > > + > > > #define dprintk(dev, fmt, arg...) \ > > > v4l2_dbg(1, debug, &dev->v4l2_dev, "%s: " fmt, __func__, ## arg) > > > > > > @@ -192,6 +198,8 @@ struct pxp_dev { > > > struct clk *clk; > > > void __iomem *mmio; > > > > > > + u32 hw_version; > > > + > > > atomic_t num_inst; > > > struct mutex dev_mutex; > > > spinlock_t irqlock; > > > @@ -1660,6 +1668,11 @@ static int pxp_soft_reset(struct pxp_dev *dev) > > > return 0; > > > } > > > > > > +static u32 pxp_read_version(struct pxp_dev *dev) > > > +{ > > > + return readl(dev->mmio + HW_PXP_VERSION); > > > +} > > > + > > > static int pxp_probe(struct platform_device *pdev) > > > { > > > struct pxp_dev *dev; > > > @@ -1705,6 +1718,11 @@ static int pxp_probe(struct platform_device *pdev) > > > goto err_clk; > > > } > > > > > > + dev->hw_version = pxp_read_version(dev); > > > + dev_info(&pdev->dev, "PXP Version %d.%d\n", > > > > As the version can't be negative, I'd use %u.%u. > > > > > + PXP_VERSION_MAJOR(dev->hw_version), > > > + PXP_VERSION_MINOR(dev->hw_version)); > > > + > > > > The driver now prints two messages at probe time, it would be nice to > > combine them, or remove the other one. That's a candidate for a future > > patch though. I would reduce the level to dev_debug. Then the version is not always printed, but it can be easily enabled if necessary for the bringup on another platform. Michael > > > > > ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev); > > > if (ret) > > > goto err_clk; _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-01-06 14:02 UTC|newest] Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-01-05 13:47 [PATCH 0/8] media: imx-pxp: add support for i.MX7D Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-05 13:47 ` [PATCH 1/8] media: dt-bindings: media: fsl-pxp: convert to yaml Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 3:18 ` Rob Herring 2023-01-06 3:18 ` Rob Herring 2023-01-06 8:23 ` Michael Tretter 2023-01-06 8:23 ` Michael Tretter 2023-01-06 11:35 ` Laurent Pinchart 2023-01-06 11:35 ` Laurent Pinchart 2023-01-06 12:34 ` Krzysztof Kozlowski 2023-01-06 12:34 ` Krzysztof Kozlowski 2023-01-05 13:47 ` [PATCH 2/8] media: imx-pxp: detect PXP version Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 11:47 ` Laurent Pinchart 2023-01-06 11:47 ` Laurent Pinchart 2023-01-06 12:28 ` Laurent Pinchart 2023-01-06 12:28 ` Laurent Pinchart 2023-01-06 14:01 ` Michael Tretter [this message] 2023-01-06 14:01 ` Michael Tretter 2023-01-06 18:40 ` Laurent Pinchart 2023-01-06 18:40 ` Laurent Pinchart 2023-01-05 13:47 ` [PATCH 3/8] media: imx-pxp: extract helper function to setup data path Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 11:59 ` Laurent Pinchart 2023-01-06 11:59 ` Laurent Pinchart 2023-01-05 13:47 ` [PATCH 4/8] media: imx-pxp: explicitly disable unused blocks Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 12:26 ` Laurent Pinchart 2023-01-06 12:26 ` Laurent Pinchart 2023-01-06 14:08 ` Michael Tretter 2023-01-06 14:08 ` Michael Tretter 2023-01-06 18:39 ` Laurent Pinchart 2023-01-06 18:39 ` Laurent Pinchart 2023-01-05 13:47 ` [PATCH 5/8] media: imx-pxp: disable LUT block Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 12:27 ` Laurent Pinchart 2023-01-06 12:27 ` Laurent Pinchart 2023-01-05 13:47 ` [PATCH 6/8] media: imx-pxp: make data_path_ctrl0 platform dependent Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 12:30 ` Laurent Pinchart 2023-01-06 12:30 ` Laurent Pinchart 2023-01-06 14:11 ` Michael Tretter 2023-01-06 14:11 ` Michael Tretter 2023-01-06 18:42 ` Laurent Pinchart 2023-01-06 18:42 ` Laurent Pinchart 2023-01-05 13:47 ` [PATCH 7/8] media: imx-pxp: add support for i.MX7D Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 12:32 ` Laurent Pinchart 2023-01-06 12:32 ` Laurent Pinchart 2023-01-05 13:47 ` [PATCH 8/8] ARM: dts: imx7d: add node for PXP Michael Tretter 2023-01-05 13:47 ` Michael Tretter 2023-01-06 12:36 ` Laurent Pinchart 2023-01-06 12:36 ` Laurent Pinchart 2023-01-06 14:36 ` Michael Tretter 2023-01-06 14:36 ` Michael Tretter 2023-01-06 18:43 ` Laurent Pinchart 2023-01-06 18:43 ` Laurent Pinchart 2023-01-06 12:41 ` [PATCH 0/8] media: imx-pxp: add support for i.MX7D Laurent Pinchart 2023-01-06 12:41 ` Laurent Pinchart 2023-01-06 14:42 ` Michael Tretter 2023-01-06 14:42 ` Michael Tretter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230106140141.GC24101@pengutronix.de \ --to=m.tretter@pengutronix.de \ --cc=devicetree@vger.kernel.org \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=laurent.pinchart@ideasonboard.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=p.zabel@pengutronix.de \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.