* [PATCH 4/8] pinctrl: imx8mq: Support building as module
From: Anson Huang @ 2020-06-05 6:34 UTC (permalink / raw)
To: aisheng.dong, festevam, shawnguo, stefan, kernel, linus.walleij,
s.hauer, linux-gpio, linux-kernel, linux-arm-kernel
Cc: Linux-imx
In-Reply-To: <1591338874-4733-1-git-send-email-Anson.Huang@nxp.com>
Support building i.MX8MQ pinctrl driver as module.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/pinctrl/freescale/Kconfig | 2 +-
drivers/pinctrl/freescale/pinctrl-imx8mq.c | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index b909719..df77e752 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -145,7 +145,7 @@ config PINCTRL_IMX8MP
Say Y here to enable the imx8mp pinctrl driver
config PINCTRL_IMX8MQ
- bool "IMX8MQ pinctrl driver"
+ tristate "IMX8MQ pinctrl driver"
depends on ARCH_MXC
select PINCTRL_IMX
help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mq.c b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
index 50aa1c0..db5b41a 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8mq.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mq.c
@@ -8,6 +8,7 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/pinctrl/pinctrl.h>
@@ -329,6 +330,7 @@ static const struct of_device_id imx8mq_pinctrl_of_match[] = {
{ .compatible = "fsl,imx8mq-iomuxc", .data = &imx8mq_pinctrl_info, },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, imx8mq_pinctrl_of_match);
static int imx8mq_pinctrl_probe(struct platform_device *pdev)
{
@@ -345,8 +347,5 @@ static struct platform_driver imx8mq_pinctrl_driver = {
.probe = imx8mq_pinctrl_probe,
};
-static int __init imx8mq_pinctrl_init(void)
-{
- return platform_driver_register(&imx8mq_pinctrl_driver);
-}
-arch_initcall(imx8mq_pinctrl_init);
+module_platform_driver(imx8mq_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/8] pinctrl: imx: Export necessary APIs for i.MX pinctrl drivers
From: Anson Huang @ 2020-06-05 6:34 UTC (permalink / raw)
To: aisheng.dong, festevam, shawnguo, stefan, kernel, linus.walleij,
s.hauer, linux-gpio, linux-kernel, linux-arm-kernel
Cc: Linux-imx
In-Reply-To: <1591338874-4733-1-git-send-email-Anson.Huang@nxp.com>
Export imx_pinctrl_probe()/imx_pinctrl_pm_ops/imx_pinctrl_sc_ipc_init()
to support i.MX SoCs' pinctrl driver to be built as module.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/pinctrl/freescale/pinctrl-imx.c | 2 ++
drivers/pinctrl/freescale/pinctrl-scu.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index cb7e0f0..f18f0d7 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -878,6 +878,7 @@ int imx_pinctrl_probe(struct platform_device *pdev,
return pinctrl_enable(ipctl->pctl);
}
+EXPORT_SYMBOL_GPL(imx_pinctrl_probe);
static int __maybe_unused imx_pinctrl_suspend(struct device *dev)
{
@@ -897,3 +898,4 @@ const struct dev_pm_ops imx_pinctrl_pm_ops = {
SET_LATE_SYSTEM_SLEEP_PM_OPS(imx_pinctrl_suspend,
imx_pinctrl_resume)
};
+EXPORT_SYMBOL_GPL(imx_pinctrl_pm_ops);
diff --git a/drivers/pinctrl/freescale/pinctrl-scu.c b/drivers/pinctrl/freescale/pinctrl-scu.c
index 23cf04b..200e875 100644
--- a/drivers/pinctrl/freescale/pinctrl-scu.c
+++ b/drivers/pinctrl/freescale/pinctrl-scu.c
@@ -41,6 +41,7 @@ int imx_pinctrl_sc_ipc_init(struct platform_device *pdev)
{
return imx_scu_get_handle(&pinctrl_ipc_handle);
}
+EXPORT_SYMBOL_GPL(imx_pinctrl_sc_ipc_init);
int imx_pinconf_get_scu(struct pinctrl_dev *pctldev, unsigned pin_id,
unsigned long *config)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 2/8] pinctrl: imx8mm: Support building as module
From: Anson Huang @ 2020-06-05 6:34 UTC (permalink / raw)
To: aisheng.dong, festevam, shawnguo, stefan, kernel, linus.walleij,
s.hauer, linux-gpio, linux-kernel, linux-arm-kernel
Cc: Linux-imx
In-Reply-To: <1591338874-4733-1-git-send-email-Anson.Huang@nxp.com>
Support building i.MX8MM pinctrl driver as module.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/pinctrl/freescale/Kconfig | 2 +-
drivers/pinctrl/freescale/pinctrl-imx8mm.c | 10 ++++------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/pinctrl/freescale/Kconfig b/drivers/pinctrl/freescale/Kconfig
index 4ca44dd..3681c4d 100644
--- a/drivers/pinctrl/freescale/Kconfig
+++ b/drivers/pinctrl/freescale/Kconfig
@@ -124,7 +124,7 @@ config PINCTRL_IMX7ULP
Say Y here to enable the imx7ulp pinctrl driver
config PINCTRL_IMX8MM
- bool "IMX8MM pinctrl driver"
+ tristate "IMX8MM pinctrl driver"
depends on ARCH_MXC
select PINCTRL_IMX
help
diff --git a/drivers/pinctrl/freescale/pinctrl-imx8mm.c b/drivers/pinctrl/freescale/pinctrl-imx8mm.c
index 6d1038a..eca1424 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx8mm.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx8mm.c
@@ -5,6 +5,7 @@
#include <linux/err.h>
#include <linux/init.h>
+#include <linux/module.h>
#include <linux/of_device.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/platform_device.h>
@@ -326,6 +327,7 @@ static const struct of_device_id imx8mm_pinctrl_of_match[] = {
{ .compatible = "fsl,imx8mm-iomuxc", .data = &imx8mm_pinctrl_info, },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, imx8mm_pinctrl_of_match);
static int imx8mm_pinctrl_probe(struct platform_device *pdev)
{
@@ -340,9 +342,5 @@ static struct platform_driver imx8mm_pinctrl_driver = {
},
.probe = imx8mm_pinctrl_probe,
};
-
-static int __init imx8mm_pinctrl_init(void)
-{
- return platform_driver_register(&imx8mm_pinctrl_driver);
-}
-arch_initcall(imx8mm_pinctrl_init);
+module_platform_driver(imx8mm_pinctrl_driver);
+MODULE_LICENSE("GPL v2");
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/8] Support i.MX8 SoCs pinctrl drivers built as module
From: Anson Huang @ 2020-06-05 6:34 UTC (permalink / raw)
To: aisheng.dong, festevam, shawnguo, stefan, kernel, linus.walleij,
s.hauer, linux-gpio, linux-kernel, linux-arm-kernel
Cc: Linux-imx
There are more and mroe requirements that SoC specific modules should be
built as module in order to support generic kernel image, such as Android
GKI concept.
This patch series supports i.MX8 SoCs pinctrl drivers to be built as module,
including i.MX8MQ/MM/MN/MP/QXP/QM/DXL SoCs.
Anson Huang (8):
pinctrl: imx: Export necessary APIs for i.MX pinctrl drivers
pinctrl: imx8mm: Support building as module
pinctrl: imx8mn: Support building as module
pinctrl: imx8mq: Support building as module
pinctrl: imx8mp: Support building as module
pinctrl: imx8qxp: Support building as module
pinctrl: imx8qm: Support building as module
pinctrl: imx8dxl: Support building as module
drivers/pinctrl/freescale/Kconfig | 14 +++++++-------
drivers/pinctrl/freescale/pinctrl-imx.c | 2 ++
drivers/pinctrl/freescale/pinctrl-imx8dxl.c | 9 +++------
drivers/pinctrl/freescale/pinctrl-imx8mm.c | 10 ++++------
drivers/pinctrl/freescale/pinctrl-imx8mn.c | 10 ++++------
drivers/pinctrl/freescale/pinctrl-imx8mp.c | 10 ++++------
drivers/pinctrl/freescale/pinctrl-imx8mq.c | 9 ++++-----
drivers/pinctrl/freescale/pinctrl-imx8qm.c | 9 +++------
drivers/pinctrl/freescale/pinctrl-imx8qxp.c | 9 +++------
drivers/pinctrl/freescale/pinctrl-scu.c | 1 +
10 files changed, 35 insertions(+), 48 deletions(-)
--
2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 10/14] media: platform: Delete redundant code for improving code quality
From: Xia Jiang @ 2020-06-05 6:41 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521154958.GI209565@chromium.org>
On Thu, 2020-05-21 at 15:49 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:29PM +0800, Xia Jiang wrote:
> > Delete unused member variables annotation.
> > Delete unused variable definition.
> > Delete redundant log print, because V4L2 debug logs already print it.
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 16 ++--------------
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h | 5 +++--
> > 2 files changed, 5 insertions(+), 16 deletions(-)
> >
>
> Thank you for the patch. Please see my comments inline.
>
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 4e64046a6854..9e59b9a51ef0 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -182,7 +182,6 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > struct mtk_jpeg_ctx *ctx, int q_type)
> > {
> > struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
> > - struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> > int i;
> >
> > memset(pix_mp->reserved, 0, sizeof(pix_mp->reserved));
> > @@ -190,7 +189,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> >
> > if (ctx->state != MTK_JPEG_INIT) {
> > mtk_jpeg_adjust_fmt_mplane(ctx, f);
> > - goto end;
> > + return 0;
> > }
> >
> > pix_mp->num_planes = fmt->colplanes;
> > @@ -210,7 +209,7 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> > if (pfmt->sizeimage == 0)
> > pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > - goto end;
> > + return 0;
> > }
> >
> > /* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > @@ -224,20 +223,9 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > u32 stride = pix_mp->width * fmt->h_sample[i] / 4;
> > u32 h = pix_mp->height * fmt->v_sample[i] / 4;
> >
> > - memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
>
> This change is not mentioned in the description. I'd suggest moving it
> to a separate patch, because it's a functional change.
>
> > pfmt->bytesperline = stride;
> > pfmt->sizeimage = stride * h;
> > }
> > -end:
> > - v4l2_dbg(2, debug, &jpeg->v4l2_dev, "wxh:%ux%u\n",
> > - pix_mp->width, pix_mp->height);
> > - for (i = 0; i < pix_mp->num_planes; i++) {
> > - v4l2_dbg(2, debug, &jpeg->v4l2_dev,
> > - "plane[%d] bpl=%u, size=%u\n",
> > - i,
> > - pix_mp->plane_fmt[i].bytesperline,
> > - pix_mp->plane_fmt[i].sizeimage);
> > - }
> > return 0;
> > }
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 64a731261214..9bbd615b1067 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -30,6 +30,9 @@
> >
> > #define MTK_JPEG_DEFAULT_SIZEIMAGE (1 * 1024 * 1024)
> >
> > +/**
> > + * enum mtk_jpeg_ctx_state - contex state of jpeg
>
> typo: s/contex/context/
>
> But I'd rephrase it to "states of the context state machine".
>
> > + */
>
> Not mentioned in the description. Also, the documentation of an enum
> should have descriptions for the values.
Done.
>
> Best regards,
> Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 07/14] media: platform: Use kernel native functions for improving code quality
From: Xia Jiang @ 2020-06-05 6:41 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521154137.GG209565@chromium.org>
On Thu, 2020-05-21 at 15:41 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:26PM +0800, Xia Jiang wrote:
>
> Thank you for the patch. Please see my comments inline.
>
> nit: I'd remove "for improving code quality" from the subject, as it's
> obvious that we don't intend to make the code quality worse. ;)
> On the contrary, I'd make it more specific, e.g.
>
> media: mtk-jpeg: Use generic rounding helpers
>
> WDYT?
Done
>
> > Use clamp() to replace mtk_jpeg_bound_align_image() and round() to
> > replace mtk_jpeg_align().
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > v8: no changes
> > ---
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 41 +++++--------------
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.h | 8 ++--
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.c | 8 ++--
> > drivers/media/platform/mtk-jpeg/mtk_jpeg_hw.h | 5 ---
> > 4 files changed, 19 insertions(+), 43 deletions(-)
> >
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index 2fa3711fdc9b..4e64046a6854 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -157,25 +157,6 @@ static struct mtk_jpeg_fmt *mtk_jpeg_find_format(struct mtk_jpeg_ctx *ctx,
> > return NULL;
> > }
> >
> > -static void mtk_jpeg_bound_align_image(u32 *w, unsigned int wmin,
> > - unsigned int wmax, unsigned int walign,
> > - u32 *h, unsigned int hmin,
> > - unsigned int hmax, unsigned int halign)
> > -{
> > - int width, height, w_step, h_step;
> > -
> > - width = *w;
> > - height = *h;
> > - w_step = 1 << walign;
> > - h_step = 1 << halign;
> > -
> > - v4l_bound_align_image(w, wmin, wmax, walign, h, hmin, hmax, halign, 0);
> > - if (*w < width && (*w + w_step) <= wmax)
> > - *w += w_step;
> > - if (*h < height && (*h + h_step) <= hmax)
> > - *h += h_step;
> > -}
> > -
> > static void mtk_jpeg_adjust_fmt_mplane(struct mtk_jpeg_ctx *ctx,
> > struct v4l2_format *f)
> > {
> > @@ -218,25 +199,25 @@ static int mtk_jpeg_try_fmt_mplane(struct v4l2_format *f,
> > if (q_type == MTK_JPEG_FMT_TYPE_OUTPUT) {
> > struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[0];
> >
> > - mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > - MTK_JPEG_MAX_WIDTH, 0,
> > - &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > - MTK_JPEG_MAX_HEIGHT, 0);
> > + pix_mp->height = clamp(pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > + MTK_JPEG_MAX_HEIGHT);
> > + pix_mp->width = clamp(pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > + MTK_JPEG_MAX_WIDTH);
> >
> > memset(pfmt->reserved, 0, sizeof(pfmt->reserved));
> > pfmt->bytesperline = 0;
> > /* Source size must be aligned to 128 */
> > - pfmt->sizeimage = mtk_jpeg_align(pfmt->sizeimage, 128);
> > + pfmt->sizeimage = round_up(pfmt->sizeimage, 128);
> > if (pfmt->sizeimage == 0)
> > pfmt->sizeimage = MTK_JPEG_DEFAULT_SIZEIMAGE;
> > goto end;
> > }
> >
> > /* type is MTK_JPEG_FMT_TYPE_CAPTURE */
> > - mtk_jpeg_bound_align_image(&pix_mp->width, MTK_JPEG_MIN_WIDTH,
> > - MTK_JPEG_MAX_WIDTH, fmt->h_align,
> > - &pix_mp->height, MTK_JPEG_MIN_HEIGHT,
> > - MTK_JPEG_MAX_HEIGHT, fmt->v_align);
> > + pix_mp->height = clamp(round_up(pix_mp->height, fmt->v_align),
> > + MTK_JPEG_MIN_HEIGHT, MTK_JPEG_MAX_HEIGHT);
> > + pix_mp->width = clamp(round_up(pix_mp->width, fmt->h_align),
> > + MTK_JPEG_MIN_WIDTH, MTK_JPEG_MAX_WIDTH);
> >
> > for (i = 0; i < fmt->colplanes; i++) {
> > struct v4l2_plane_pix_format *pfmt = &pix_mp->plane_fmt[i];
> > @@ -751,8 +732,8 @@ static void mtk_jpeg_set_dec_src(struct mtk_jpeg_ctx *ctx,
> > {
> > bs->str_addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> > bs->end_addr = bs->str_addr +
> > - mtk_jpeg_align(vb2_get_plane_payload(src_buf, 0), 16);
> > - bs->size = mtk_jpeg_align(vb2_plane_size(src_buf, 0), 128);
> > + round_up(vb2_get_plane_payload(src_buf, 0), 16);
> > + bs->size = round_up(vb2_plane_size(src_buf, 0), 128);
> > }
> >
> > static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > index 999bd1427809..28e9b30ad5c3 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.h
> > @@ -21,10 +21,10 @@
> > #define MTK_JPEG_FMT_TYPE_OUTPUT 1
> > #define MTK_JPEG_FMT_TYPE_CAPTURE 2
> >
> > -#define MTK_JPEG_MIN_WIDTH 32
> > -#define MTK_JPEG_MIN_HEIGHT 32
> > -#define MTK_JPEG_MAX_WIDTH 8192
> > -#define MTK_JPEG_MAX_HEIGHT 8192
> > +#define MTK_JPEG_MIN_WIDTH 32U
> > +#define MTK_JPEG_MIN_HEIGHT 32U
> > +#define MTK_JPEG_MAX_WIDTH 8192U
> > +#define MTK_JPEG_MAX_HEIGHT 8192U
>
> This change is not mentioned in the commit message. It should go to a
> separate patch, possibly merged with other really minor stylistic changes
> like this, e.g. patch 08/14.
Done
>
> Otherwise the patch looks good, so after addressing the above minor changes
> please feel free to add
>
> Reviewed-by: Tomasz Figa <tfiga@chromium.org>
>
> Best regards,
> Tomasz
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU
From: Jassi Brar @ 2020-06-05 6:30 UTC (permalink / raw)
To: Sudeep Holla
Cc: Rob Herring, Arnd Bergmann, Devicetree List, Viresh Kumar,
Linux Kernel Mailing List, Bjorn Andersson, Frank Rowand,
linux-arm-kernel
In-Reply-To: <20200605045645.GD12397@bogus>
On Thu, Jun 4, 2020 at 11:56 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> > >> bash-1526 [000] 1149.472553: scmi_xfer_begin: transfer_id=1538 msg_id=6 protocol_id=21 seq=0 poll=0
> > >> <idle>-0 [001] 1149.472733: scmi_xfer_begin: transfer_id=1539 msg_id=7 protocol_id=19 seq=1 poll=1
> > >
> > Here another request is started before the first is finished.
>
> Ah, the prints are when the client requested. It is not when the mailbox
> started it. So this just indicates the beginning of the transfer from the
> client.
>
There maybe condition on a sensor read to finish within 1ms, but there
is no condition for the read to _start_ at this very moment (usually
there are sleeps in the path to sensor requests).
> > If you want this to work you have to serialise access to the single
> > physical channel and/or run the remote firmware in asynchronous mode -
> > that is, the firmware ack the bit as soon as it sees and starts
> > working in the background, so that we return in ~3usec always, while
> > the data returns whenever it is ready.
>
> Yes it does that for few requests like DVFS while it uses synchronous
> mode for few others. While ideally I agree everything in asynchronous
> most is better, I don't know there may be reasons for such design.
>
The reason is that, if the firmware works asynchronously, every call
would return in ~3usec and you won't think you need virtual channels.
You have shared only 'bad' log without serialising access. Please
share log after serialising access to the channel and the 'good' log
with virtual channels. That should put the topic to rest.
thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 00/11] Add support for Kontron sl28cpld
From: Lee Jones @ 2020-06-05 6:13 UTC (permalink / raw)
To: Michael Walle
Cc: devicetree, Linus Walleij, Thierry Reding, Jason Cooper,
Andy Shevchenko, Marc Zyngier, Bartosz Golaszewski,
Uwe Kleine-König, Guenter Roeck, linux-pwm, Jean Delvare,
linux-watchdog, linux-gpio, Mark Brown, Thomas Gleixner,
Wim Van Sebroeck, linux-arm-kernel, linux-hwmon,
Greg Kroah-Hartman, linux-kernel, Li Yang, Rob Herring, Shawn Guo
In-Reply-To: <20200604211039.12689-1-michael@walle.cc>
On Thu, 04 Jun 2020, Michael Walle wrote:
> The Kontron sl28cpld is a board management chip providing gpio, pwm, fan
> monitoring and an interrupt controller. For now this controller is used on
> the Kontron SMARC-sAL28 board. But because of its flexible nature, it
> might also be used on other boards in the future. The individual blocks
> (like gpio, pwm, etc) are kept intentionally small. The MFD core driver
> then instantiates different (or multiple of the same) blocks. It also
> provides the register layout so it might be updated in the future without a
> device tree change; and support other boards with a different layout or
> functionalities.
>
> See also [1] for more information.
>
> This is my first take of a MFD driver. I don't know whether the subsystem
> maintainers should only be CCed on the patches which affect the subsystem
> or on all patches for this series. I've chosen the latter so you can get a
> more complete picture.
You chose wisely. :)
> [1] https://lore.kernel.org/linux-devicetree/0e3e8204ab992d75aa07fc36af7e4ab2@walle.cc/
>
> Changes since v3:
> - use of_platform_populate() to populate internal devices using the
> internal register offsets as unit-addresses
> - because we don't use mfd_cells anymore, we cannot use IORESOURCE_REG,
> but instead parse the reg property in each individual driver
> - dropped the following patches because they were already merged:
> gpiolib: Introduce gpiochip_irqchip_add_domain()
> gpio: add a reusable generic gpio_chip using regmap
> - dropped the following patches because they are no longer needed:
> include/linux/ioport.h: add helper to define REG resource constructs
> mfd: mfd-core: Don't overwrite the dma_mask of the child device
> mfd: mfd-core: match device tree node against reg property
> - rephrase commit messages, as suggested by Thomas Gleixner
It's great to have this changelog overview.
However it's equally, if not arguably more important to have a more
fine grained changelog in each of the patches, usually placed between
the '---' and the diff stat.
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 05/14] media: platform: Improve power on and power off flow
From: Xia Jiang @ 2020-06-05 6:03 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521152253.GE209565@chromium.org>
On Thu, 2020-05-21 at 15:22 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:24PM +0800, Xia Jiang wrote:
> > Call pm_runtime_get_sync() before starting a frame and then
> > pm_runtime_put() after completing it. This can save power for the time
> > between processing two frames.
> >
> > Signed-off-by: Xia Jiang <xia.jiang@mediatek.com>
> > ---
> > .../media/platform/mtk-jpeg/mtk_jpeg_core.c | 27 +++++--------------
> > 1 file changed, 6 insertions(+), 21 deletions(-)
> >
>
> Thank you for the patch. Please see my comments inline.
>
> > diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > index a536fa95b3d6..dd5cadd101ef 100644
> > --- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
> > @@ -710,23 +710,6 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> > return v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> > }
> >
> > -static int mtk_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
> > -{
> > - struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > - struct vb2_v4l2_buffer *vb;
> > - int ret = 0;
> > -
> > - ret = pm_runtime_get_sync(ctx->jpeg->dev);
> > - if (ret < 0)
> > - goto err;
> > -
> > - return 0;
> > -err:
> > - while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > - v4l2_m2m_buf_done(vb, VB2_BUF_STATE_QUEUED);
> > - return ret;
> > -}
> > -
> > static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> > {
> > struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> > @@ -751,8 +734,6 @@ static void mtk_jpeg_stop_streaming(struct vb2_queue *q)
> >
> > while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > -
> > - pm_runtime_put_sync(ctx->jpeg->dev);
> > }
> >
> > static const struct vb2_ops mtk_jpeg_qops = {
> > @@ -761,7 +742,6 @@ static const struct vb2_ops mtk_jpeg_qops = {
> > .buf_queue = mtk_jpeg_buf_queue,
> > .wait_prepare = vb2_ops_wait_prepare,
> > .wait_finish = vb2_ops_wait_finish,
> > - .start_streaming = mtk_jpeg_start_streaming,
> > .stop_streaming = mtk_jpeg_stop_streaming,
> > };
> >
> > @@ -812,7 +792,7 @@ static void mtk_jpeg_device_run(void *priv)
> > struct mtk_jpeg_src_buf *jpeg_src_buf;
> > struct mtk_jpeg_bs bs;
> > struct mtk_jpeg_fb fb;
> > - int i;
> > + int i, ret;
> >
> > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > @@ -832,6 +812,10 @@ static void mtk_jpeg_device_run(void *priv)
> > return;
> > }
> >
> > + ret = pm_runtime_get_sync(jpeg->dev);
> > + if (ret < 0)
> > + goto dec_end;
> > +
> > mtk_jpeg_set_dec_src(ctx, &src_buf->vb2_buf, &bs);
> > if (mtk_jpeg_set_dec_dst(ctx, &jpeg_src_buf->dec_param, &dst_buf->vb2_buf, &fb))
> > goto dec_end;
> > @@ -957,6 +941,7 @@ static irqreturn_t mtk_jpeg_dec_irq(int irq, void *priv)
> > v4l2_m2m_buf_done(src_buf, buf_state);
> > v4l2_m2m_buf_done(dst_buf, buf_state);
> > v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > + pm_runtime_put_sync(ctx->jpeg->dev);
>
> The _sync variant explicitly waits until the asynchronous PM operation
> completes. This is usually undesired, because the CPU stays blocked for
> no good reason. In this context it is actually a bug, because this is an
> interrupt handler and it's not allowed to sleep. I wonder why this
> actually didn't crash in your testing. Please change to the regular
> pm_runtime_put().
Done.
>
> Best regards,
> Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 1/3] dma-direct: provide the ability to reserve per-numa CMA
From: Song Bao Hua (Barry Song) @ 2020-06-05 6:04 UTC (permalink / raw)
To: Dan Carpenter, kbuild@lists.01.org, hch@lst.de,
m.szyprowski@samsung.com, robin.murphy@arm.com,
catalin.marinas@arm.com
Cc: kbuild-all@lists.01.org, lkp@intel.com, John Garry,
linux-kernel@vger.kernel.org, Linuxarm,
iommu@lists.linux-foundation.org, Jonathan Cameron,
linux-arm-kernel@lists.infradead.org, Dan Carpenter
In-Reply-To: <20200604113631.GP30374@kadam>
> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Thursday, June 4, 2020 11:37 PM
> To: kbuild@lists.01.org; Song Bao Hua (Barry Song)
> <song.bao.hua@hisilicon.com>; hch@lst.de; m.szyprowski@samsung.com;
> robin.murphy@arm.com; catalin.marinas@arm.com
> Cc: lkp@intel.com; Dan Carpenter <error27@gmail.com>;
> kbuild-all@lists.01.org; iommu@lists.linux-foundation.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Linuxarm
> <linuxarm@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; John Garry <john.garry@huawei.com>
> Subject: Re: [PATCH 1/3] dma-direct: provide the ability to reserve per-numa
> CMA
>
> Hi Barry,
>
> url:
> https://github.com/0day-ci/linux/commits/Barry-Song/support-per-numa-CM
> A-for-ARM-server/20200603-104821
> base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
> for-next/core
> config: x86_64-randconfig-m001-20200603 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Dan, thanks! Good catch!
as this patch has not been in mainline yet, is it correct to add these "reported-by" in patch v2?
Barry
>
> smatch warnings:
> kernel/dma/contiguous.c:274 dma_alloc_contiguous() warn: variable
> dereferenced before check 'dev' (see line 272)
>
> #
> https://github.com/0day-ci/linux/commit/adb919e972c1cac3d8b11905d525
> 8d23d3aac6a4
> git remote add linux-review https://github.com/0day-ci/linux git remote
> update linux-review git checkout
> adb919e972c1cac3d8b11905d5258d23d3aac6a4
> vim +/dev +274 kernel/dma/contiguous.c
>
> b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 267 struct page *dma_alloc_contiguous(struct device *dev,
> size_t size, gfp_t gfp)
> b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 268 {
> 90ae409f9eb3bc kernel/dma/contiguous.c Christoph Hellwig
> 2019-08-20 269 size_t count = size >> PAGE_SHIFT;
> b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 270 struct page *page = NULL;
> bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 271 struct cma *cma = NULL;
> adb919e972c1ca kernel/dma/contiguous.c Barry Song
> 2020-06-03 @272 int nid = dev_to_node(dev);
>
> ^^^ Dereferenced inside function.
>
> bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 273
> bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 @274 if (dev && dev->cma_area)
>
> ^^^ Too late.
>
> bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 275 cma = dev->cma_area;
> adb919e972c1ca kernel/dma/contiguous.c Barry Song
> 2020-06-03 276 else if ((nid != NUMA_NO_NODE) &&
> dma_contiguous_pernuma_area[nid]
> adb919e972c1ca kernel/dma/contiguous.c Barry Song
> 2020-06-03 277 && !(gfp & (GFP_DMA | GFP_DMA32)))
> adb919e972c1ca kernel/dma/contiguous.c Barry Song
> 2020-06-03 278 cma = dma_contiguous_pernuma_area[nid];
> bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 279 else if (count > 1)
> bd2e75633c8012 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 280 cma = dma_contiguous_default_area;
> b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 281
> b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 282 /* CMA can be used only in the context which permits
> sleeping */
> b1d2dc009dece4 kernel/dma/contiguous.c Nicolin Chen
> 2019-05-23 283 if (cma && gfpflags_allow_blocking(gfp)) {
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v8 04/14] media: platform: Change the fixed device node number to unfixed value
From: Xia Jiang @ 2020-06-05 6:02 UTC (permalink / raw)
To: Tomasz Figa
Cc: drinkcat, devicetree, mojahsu, srv_heupstream, Rick Chang,
senozhatsky, linux-kernel, maoguang.meng, Mauro Carvalho Chehab,
sj.huang, Rob Herring, Matthias Brugger, Hans Verkuil,
linux-mediatek, Marek Szyprowski, linux-arm-kernel, linux-media
In-Reply-To: <20200521135937.GD209565@chromium.org>
On Thu, 2020-05-21 at 13:59 +0000, Tomasz Figa wrote:
> Hi Xia,
>
> On Fri, Apr 03, 2020 at 05:40:23PM +0800, Xia Jiang wrote:
> > Change device node number from 3 to -1 because that the driver will
> > also support jpeg encoder.
> >
>
> Thanks for the patch. The change is correct, but I think the commit
> message doesn't really explain the real reason for it. Perhaps something
> like
>
> "The driver can be instantiated multiple times, e.g. for a decoder and
> an encoder. Moreover, other drivers could coexist on the same system.
> This makes the static video node number assignment pointless, so switch
> to automatic assignment instead."
>
> WDYT?
Dear Tomasz,
Thanks for your advice.I have changed it in new v9 .
Best Regards,
Xia Jiang
>
> Best regards,
> Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [soc:for-next] BUILD SUCCESS 28107944fb709b04afc82ca626f334a490571e28
From: kernel test robot @ 2020-06-05 5:33 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: arm, linux-arm-kernel
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
branch HEAD: 28107944fb709b04afc82ca626f334a490571e28 soc: document merges
elapsed time: 482m
configs tested: 103
configs skipped: 7
The following configs have been built successfully.
More configs may be tested in the coming days.
arm defconfig
arm allyesconfig
arm allmodconfig
arm allnoconfig
arm64 allyesconfig
arm64 defconfig
arm64 allmodconfig
arm64 allnoconfig
mips ar7_defconfig
csky alldefconfig
sh shx3_defconfig
mips cavium_octeon_defconfig
um x86_64_defconfig
ia64 tiger_defconfig
arc nps_defconfig
arc tb10x_defconfig
arm spear13xx_defconfig
nios2 defconfig
mips jmr3927_defconfig
alpha defconfig
powerpc mpc866_ads_defconfig
mips workpad_defconfig
s390 alldefconfig
sh sh7757lcr_defconfig
m68k q40_defconfig
sh sh2007_defconfig
h8300 defconfig
sh ul2_defconfig
i386 allnoconfig
i386 allyesconfig
i386 defconfig
i386 debian-10.3
ia64 allmodconfig
ia64 defconfig
ia64 allnoconfig
ia64 allyesconfig
m68k allmodconfig
m68k allnoconfig
m68k sun3_defconfig
m68k defconfig
m68k allyesconfig
nds32 defconfig
nds32 allnoconfig
csky allyesconfig
csky defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
h8300 allmodconfig
xtensa defconfig
arc defconfig
arc allyesconfig
sh allmodconfig
sh allnoconfig
microblaze allnoconfig
nios2 allyesconfig
openrisc defconfig
c6x allyesconfig
c6x allnoconfig
openrisc allyesconfig
mips allyesconfig
mips allnoconfig
mips allmodconfig
parisc allnoconfig
parisc defconfig
parisc allyesconfig
parisc allmodconfig
powerpc defconfig
powerpc allyesconfig
powerpc rhel-kconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a002-20200605
x86_64 randconfig-a001-20200605
x86_64 randconfig-a006-20200605
x86_64 randconfig-a003-20200605
x86_64 randconfig-a004-20200605
x86_64 randconfig-a005-20200605
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
s390 allyesconfig
s390 allnoconfig
s390 allmodconfig
s390 defconfig
sparc allyesconfig
sparc defconfig
sparc64 defconfig
sparc64 allnoconfig
sparc64 allyesconfig
sparc64 allmodconfig
um allnoconfig
um defconfig
um allmodconfig
um allyesconfig
x86_64 rhel
x86_64 rhel-7.6
x86_64 rhel-7.6-kselftests
x86_64 rhel-7.2-clear
x86_64 lkp
x86_64 fedora-25
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] irqchip/gic-v4.1: Use readx_poll_timeout_atomic() to fix sleep in atomic
From: Zenghui Yu @ 2020-06-05 5:23 UTC (permalink / raw)
To: linux-kernel, linux-arm-kernel, kvmarm
Cc: jason, maz, wangjingyi11, Zenghui Yu, wanghaibin.wang, tglx
readx_poll_timeout() can sleep if @sleep_us is specified by the caller,
and is therefore unsafe to be used inside the atomic context, which is
this case when we use it to poll the GICR_VPENDBASER.Dirty bit in
irq_set_vcpu_affinity() callback.
Let's convert to its atomic version instead which helps to get the v4.1
board back to life!
Fixes: 96806229ca03 ("irqchip/gic-v4.1: Add support for VPENDBASER's Dirty+Valid signaling")
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
drivers/irqchip/irq-gic-v3-its.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index cd685f521c77..6a5a87fc4601 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3797,10 +3797,10 @@ static void its_wait_vpt_parse_complete(void)
if (!gic_rdists->has_vpend_valid_dirty)
return;
- WARN_ON_ONCE(readq_relaxed_poll_timeout(vlpi_base + GICR_VPENDBASER,
- val,
- !(val & GICR_VPENDBASER_Dirty),
- 10, 500));
+ WARN_ON_ONCE(readq_relaxed_poll_timeout_atomic(vlpi_base + GICR_VPENDBASER,
+ val,
+ !(val & GICR_VPENDBASER_Dirty),
+ 10, 500));
}
static void its_vpe_schedule(struct its_vpe *vpe)
--
2.19.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC] dt-bindings: mailbox: add doorbell support to ARM MHU
From: Sudeep Holla @ 2020-06-05 4:56 UTC (permalink / raw)
To: Jassi Brar
Cc: Rob Herring, Arnd Bergmann, Devicetree List, Viresh Kumar,
Linux Kernel Mailing List, Bjorn Andersson, Sudeep Holla,
Frank Rowand, linux-arm-kernel
In-Reply-To: <CABb+yY27Ngb0C-onkU2qyt=uKgG4iVrcv8hGkC+anypQbTRA1w@mail.gmail.com>
On Thu, Jun 04, 2020 at 10:15:55AM -0500, Jassi Brar wrote:
> On Thu, Jun 4, 2020 at 4:20 AM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Wed, Jun 03, 2020 at 01:32:42PM -0500, Jassi Brar wrote:
> > > On Wed, Jun 3, 2020 at 1:04 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > On Fri, May 29, 2020 at 09:37:58AM +0530, Viresh Kumar wrote:
> > > > > On 28-05-20, 13:20, Rob Herring wrote:
> > > > > > Whether Linux
> > > > > > requires serializing mailbox accesses is a separate issue. On that side,
> > > > > > it seems silly to not allow driving the h/w in the most efficient way
> > > > > > possible.
> > > > >
> > > > > That's exactly what we are trying to say. The hardware allows us to
> > > > > write all 32 bits in parallel, without any hardware issues, why
> > > > > shouldn't we do that ? The delay (which Sudeep will find out, he is
> > > > > facing issues with hardware access because of lockdown right now)
> > > >
> > > > OK, I was able to access the setup today. I couldn't reach a point
> > > > where I can do measurements as the system just became unusable with
> > > > one physical channel instead of 2 virtual channels as in my patches.
> > > >
> > > > My test was simple. Switch to schedutil and read sensors periodically
> > > > via sysfs.
> > > >
> > > > arm-scmi firmware:scmi: message for 1 is not expected!
> > > >
> > > This sounds like you are not serialising requests on a shared channel.
> > > Can you please also share the patch?
> >
> > OK, I did try with a small patch initially and then realised we must hit
> > issue with mainline as is. Tried and the behaviour is exact same. All
> > I did is removed my patches and use bit[0] as the signal. It doesn't
> > matter as writes to the register are now serialised. Oh, the above
> > message comes when OS times out in advance while firmware continues to
> > process the old request and respond.
> >
> > The trace I sent gives much better view of what's going on.
> >
> BTW, you didn't even share 'good' vs 'bad' log for me to actually see
> if the api lacks.
>
> Let us look closely ...
>
> >> bash-1019 [005] 1149.452340: scmi_xfer_begin:
> transfer_id=1537 msg_id=7 protocol_id=19 seq=0 poll=1
> >> bash-1019 [005] 1149.452407: scmi_xfer_end:
> transfer_id=1537 msg_id=7 protocol_id=19 seq=0 status=0
> >
> This round trip took 67usecs. (log shows some at even 3usecs)
> That includes mailbox api overhead, memcpy and the time taken by
> remote to respond.
This is DVFS request which firmware acknowledges quickly and expected
to at most 100us.
> So the api is definitely capable of much faster transfers than you require.
>
I am not complaining about that. The delay is mostly due to the load on
the mailbox and parallelising helps is the focus here.
> >> bash-1526 [000] 1149.472553: scmi_xfer_begin: transfer_id=1538 msg_id=6 protocol_id=21 seq=0 poll=0
> >> <idle>-0 [001] 1149.472733: scmi_xfer_begin: transfer_id=1539 msg_id=7 protocol_id=19 seq=1 poll=1
> >
> Here another request is started before the first is finished.
Ah, the prints are when the client requested. It is not when the mailbox
started it. So this just indicates the beginning of the transfer from the
client. I must have mentioned that earlier. Some request timeout before
being picked up by mailbox if the previous request is not acknowledge
quickly. E.g. Say a sensor command started which may take upto 1ms,
almost 5-6 DVFS request after that will timeout.
> If you want this to work you have to serialise access to the single
> physical channel and/or run the remote firmware in asynchronous mode -
> that is, the firmware ack the bit as soon as it sees and starts
> working in the background, so that we return in ~3usec always, while
> the data returns whenever it is ready.
Yes it does that for few requests like DVFS while it uses synchronous
mode for few others. While ideally I agree everything in asynchronous
most is better, I don't know there may be reasons for such design. Also
the solution given is to use different bits as independent channels
which hardware allows. Anyways it's open source SCP project[1].
--
Regards,
Sudeep
[1] https://github.com/ARM-software/SCP-firmware
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] arm64: Override SPSR.SS when single-stepping is enabled
From: Luis Machado @ 2020-06-05 4:50 UTC (permalink / raw)
To: Will Deacon, Keno Fischer
Cc: Mark Rutland, kernel-team, stable, linux-arm-kernel
In-Reply-To: <20200604083210.GC30155@willie-the-truck>
Hi,
On 6/4/20 5:32 AM, Will Deacon wrote:
> Hi Keno,
>
> Cheers for the really helpful explanation. I have a bunch of
> questions/comments, since it's not very often that somebody shows up who
> understands how this is supposed to work and so I'd like to take advantage
> of that!
>
> On Wed, Jun 03, 2020 at 12:56:24PM -0400, Keno Fischer wrote:
>> On Wed, Jun 3, 2020 at 11:53 AM Will Deacon <will@kernel.org> wrote:
>>>> However, at the same time as changing this, we should probably make sure
>>>> to enable the syscall exit pseudo-singlestep trap (similar issue as the other
>>>> patch I had sent for the signal pseudo-singlestep trap), since otherwise
>>>> ptracers might get confused about the lack of singlestep trap during a
>>>> singlestep -> seccomp -> singlestep path (which would give one trap
>>>> less with this patch than before).
>>>
>>> Hmm, I don't completely follow your example. Please could you spell it
>>> out a bit more? I fast-forward the stepping state machine on sigreturn,
>>> which I thought would be sufficient. Perhaps you're referring to a variant
>>> of the situation mentioned by Mark, which I didn't think could happen
>>> with ptrace [2].
>>
>> Sure suppose we have code like the following:
>>
>> 0x0: svc #0
>> 0x4: str x0, [x7]
>> ...
>>
>> Then, if there's a seccomp filter active that just does
>> SECCOMP_RET_TRACE of everything, right now we get traps:
>>
>> <- (ip: 0x0)
>> -> PTRACE_SINGLESTEP
>> <- (ip: 0x4 - seccomp trap)
>> -> PTRACE_SINGLESTEP
>> <- SIGTRAP (ip: 0x4 - TRAP_TRACE trap)
>> -> PTRACE_SINGLESTEP
>> <- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
>>
>> With your proposed patch, we instead get
>> <- (ip: 0x0)
>> -> PTRACE_SINGLESTEP
>> <- (ip: 0x4 - seccomp trap)
>> -> PTRACE_SINGLESTEP
>> <- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
>
> Urgh, and actually, I think this is *only* the case if the seccomp
> handler actually changes a register in the target, right?
>
> In which case, your proposed patch should probably do something like:
>
> if (dir == PTRACE_SYSCALL_EXIT) {
> bool stepping = test_thread_flag(TIF_SINGLESTEP);
>
> tracehook_report_syscall_exit(regs, stepping);
> user_rewind_single_step(regs);
> }
>
> otherwise I think we could get a spurious SIGTRAP on return to userspace.
> What do you think?
>
> This has also got me thinking about your other patch to report a pseudo-step
> exception on entry to a signal handler:
>
> https://lore.kernel.org/r/20200524043827.GA33001@juliacomputing.com
>
> Although we don't actually disarm the step logic there (and so you might
> expect a spurious SIGTRAP on the second instruction of the handler), I
> think it's ok because the tracer will either do PTRACE_SINGLESTEP (and
> rearm the state machine) or PTRACE_CONT (and so stepping will be
> disabled). Do you agree?
>
>> This is problematic, because the ptracer may want to inspect the
>> result of the syscall instruction. On other architectures, this
>> problem is solved with a pseudo-singlestep trap that gets executed
>> if you resume from a syscall-entry-like trap with PTRACE_SINGLESTEP.
>> See the below patch for the change I'm proposing. There is a slight
>> issue with that patch, still: It now makes the x7 issue apply to the
>> singlestep trap at exit, so we should do the patch to fix that issue
>> before we apply that change (or manually check for this situation
>> and issue the pseudo-singlestep trap manually).
>
> I don't see the dependency on the x7 issue; x7 is nobbled on syscall entry,
> so it will be nobbled in the psuedo-step trap as well as the hardware step
> trap on syscall return. I'd also like to backport this to stable, without
> having to backport an optional extension to the ptrace API for preserving
> x7. Or are you saying that the value of x7 should be PTRACE_SYSCALL_ENTER
> for the pseudo trap? That seems a bit weird to me, but then this is all
> weird anyway.
>
>> My proposed patch below also changes
>>
>> <- (ip: 0x0)
>> -> PTRACE_SYSCALL
>> <- (ip: 0x4 - syscall entry trap)
>> -> PTRACE_SINGLESTEP
>> <- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
>>
>> to
>>
>> <- (ip: 0x0)
>> -> PTRACE_SYSCALL
>> <- (ip: 0x4 - syscall entry trap)
>> -> PTRACE_SINGLESTEP
>> <- SIGTRAP (ip: 0x4 - pseudo-singlestep exit trap)
>> -> PTRACE_SINGLESTEP
>> <- SIGTRAP (ip: 0x8 - TRAP_TRACE trap)
>>
>> But I consider that a bugfix, since that's how other architectures
>> behave and I was going to send in this patch for that reason anyway
>> (since this was another one of the aarch64 ptrace quirks we had to
>> work around).
>
> I think that's still the case with my addition above, so that's good.
> Any other quirks you ran into that we should address here? Now that I have
> this stuff partially paged-in, it would be good to fix a bunch of this
> at once. I can send out a v2 which includes the two patches from you
> once we're agreed on the details.
Since we're discussing arm64 ptrace/kernel quirks, I'm gonna go ahead
and describe a strange behavior on arm64 that I could not reproduce on
x86, for example. I apologize for hijacking the thread if this is a
non-issue or not related.
This is something I noticed when single-stepping over fork and vfork
syscalls in GDB, so handling of PTRACE_EVENT_FORK, PTRACE_EVENT_VFORK
and PTRACE_EVENT_VFORK_DONE.
The situation seems to happen more reliably with vforks since it is a
two stage operation with VFORK and VFORK_DONE.
Suppose we're stopped at a vfork syscall instruction and that the child
we spawn will exit immediately. If we attempt to single-step that
particular instruction, this is what happens for arm64:
--
[Step over vfork syscall]
ptrace(PTRACE_SINGLESTEP, 63049, 0x1, SIG_0) = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=63049,
si_uid=13595, si_status=SIGTRAP, si_utime=0, si_stime=0} ---
[vfork event for child 63052]
ptrace(PTRACE_GETEVENTMSG, 63049, NULL, [63052]) = 0
...
[Detach child]
ptrace(PTRACE_DETACH, 63052, NULL, SIG_0) = 0
ptrace(PTRACE_CONT, 63049, 0x1, SIG_0) = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=63049,
si_uid=13595, si_status=SIGTRAP, si_utime=0, si_stime=0} ---
...
ptrace(PTRACE_SINGLESTEP, 63049, 0x1, SIG_0) = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=63049,
si_uid=13595, si_status=SIGCHLD, si_utime=0, si_stime=0} ---
--
For x86-64, we have this:
--
[Step over vfork syscall]
ptrace(PTRACE_SINGLESTEP, 13484, 0x1, SIG_0) = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=13484,
si_uid=1000, si_status=SIGTRAP, si_utime=0, si_stime=0} ---
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=13493,
si_uid=1000, si_status=SIGSTOP, si_utime=0, si_stime=0} ---
[vfork event for child 13493]
ptrace(PTRACE_GETEVENTMSG, 13484, NULL, [13493]) = 0
...
[Detach child]
ptrace(PTRACE_DETACH, 13493, NULL, SIG_0) = 0
ptrace(PTRACE_CONT, 13484, 0x1, SIG_0) = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=13484,
si_uid=1000, si_status=SIGTRAP, si_utime=0, si_stime=0} ---
...
ptrace(PTRACE_SINGLESTEP, 13484, 0x1, SIG_0) = 0
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_TRAPPED, si_pid=13484,
si_uid=1000, si_status=SIGTRAP, si_utime=0, si_stime=0} ---
--
There are a couple things off:
1 - x86-64 seems to get an extra SIGSTOP when we single-step over the
vfork syscall, though this doesn't seem to do any harm.
2 - This is the one that throws GDB off. In the last single-step
request, arm64 gets a SIGCHLD instead of the SIGTRAP x86-64 gets.
I did some experiments with it, and it seems the last SIGCHLD is more
prone to being delivered (instead of a SIGTRAP) if we put some load on
the machine (by firing off processes or producing a lot of screen output
for example).
Does this ring any bells? I suppose signal delivery order is not
guaranteed in this context, but x86-64 seems to deliver them
consistently in the same order.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
From: Michael Kao @ 2020-06-05 3:50 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Mark Rutland, devicetree, srv_heupstream, linux-pm, linux-kernel,
Eduardo Valentin, Rob Herring, linux-mediatek, hsinyi,
Matthias Brugger, Zhang Rui, linux-arm-kernel
In-Reply-To: <1afbf412-fbeb-8abe-66d8-bd7ac4e9dd83@linaro.org>
On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote:
> On 23/03/2020 13:15, Michael Kao wrote:
> > From: "michael.kao" <michael.kao@mediatek.com>
> >
> > The driver of thermal and svs will use the
> > same register for the project which should select
> > bank before reading sensor value.
>
> Here there is a design problem AFAICT. The sensor should not be using
> external locks.
>
The PTPCORESEL is a common register used by svs and thermal.
The thermal need to ensure PTPCORESEL register will not be changed by
svs when thermal switch bank to read raw data of temperature.
So we use svs_lock to make sure there is no conflict between the two
drivers.
>
> > Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > ---
> > drivers/thermal/mtk_thermal.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > index 9eaca432920e..594ad4f0f8cd 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -22,6 +22,7 @@
> > #include <linux/thermal.h>
> > #include <linux/reset.h>
> > #include <linux/types.h>
> > +#include <linux/power/mtk_svs.h>
> >
> > /* AUXADC Registers */
> > #define AUXADC_CON1_SET_V 0x008
> > @@ -262,7 +263,7 @@ struct mtk_thermal {
> > struct clk *clk_peri_therm;
> > struct clk *clk_auxadc;
> > /* lock: for getting and putting banks */
> > - struct mutex lock;
> > + unsigned long flags;
> >
> > /* Calibration values */
> > s32 adc_ge;
> > @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
> > u32 val;
> >
> > if (mt->conf->need_switch_bank) {
> > - mutex_lock(&mt->lock);
> > + mt->flags = claim_mtk_svs_lock();
> >
> > val = readl(mt->thermal_base + PTPCORESEL);
> > val &= ~0xf;
> > @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
> > struct mtk_thermal *mt = bank->mt;
> >
> > if (mt->conf->need_switch_bank)
> > - mutex_unlock(&mt->lock);
> > + release_mtk_svs_lock(mt->flags);
> > }
> >
> > /**
> > @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> > if (ret)
> > return ret;
> >
> > - mutex_init(&mt->lock);
> > -
> > mt->dev = &pdev->dev;
> >
> > auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
> >
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] can: xilinx_can: handle failure cases of pm_runtime_get_sync
From: Navid Emamdoost @ 2020-06-05 3:32 UTC (permalink / raw)
To: Appana Durga Kedareswara rao, Naga Sureshkumar Relli,
Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
Michal Simek, linux-can, netdev, linux-arm-kernel, linux-kernel
Cc: Navid Emamdoost, emamd001, kjlu, wu000273, smccaman
Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/net/can/xilinx_can.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index c1dbab8c896d..748ff70f6a7b 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1391,7 +1391,7 @@ static int xcan_open(struct net_device *ndev)
if (ret < 0) {
netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
__func__, ret);
- return ret;
+ goto err;
}
ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
@@ -1475,6 +1475,7 @@ static int xcan_get_berr_counter(const struct net_device *ndev,
if (ret < 0) {
netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
__func__, ret);
+ pm_runtime_put(priv->dev);
return ret;
}
@@ -1789,7 +1790,7 @@ static int xcan_probe(struct platform_device *pdev)
if (ret < 0) {
netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
__func__, ret);
- goto err_pmdisable;
+ goto err_disableclks;
}
if (priv->read_reg(priv, XCAN_SR_OFFSET) != XCAN_SR_CONFIG_MASK) {
@@ -1824,7 +1825,6 @@ static int xcan_probe(struct platform_device *pdev)
err_disableclks:
pm_runtime_put(priv->dev);
-err_pmdisable:
pm_runtime_disable(&pdev->dev);
err_free:
free_candev(ndev);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [V9, 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver
From: Dongchun Zhu @ 2020-06-05 3:19 UTC (permalink / raw)
To: Sakari Ailus
Cc: mark.rutland, drinkcat, andriy.shevchenko, srv_heupstream,
devicetree, linus.walleij, shengnan.wang, tfiga, bgolaszewski,
sj.huang, robh+dt, linux-mediatek, louis.kuo, matthias.bgg,
bingbu.cao, mchehab, linux-arm-kernel, linux-media
In-Reply-To: <20200604092616.GJ16711@paasikivi.fi.intel.com>
Hi Sakari,
On Thu, 2020-06-04 at 12:26 +0300, Sakari Ailus wrote:
> Hi Dongchun,
>
> On Thu, Jun 04, 2020 at 10:14:05AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz, Sakari, and sirs,
> >
> > Could anyone help to review this patch?
> >
> > On Sat, 2020-05-23 at 16:41 +0800, Dongchun Zhu wrote:
> > > Add a V4L2 sub-device driver for OV02A10 image sensor.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/media/i2c/Kconfig | 13 +
> > > drivers/media/i2c/Makefile | 1 +
> > > drivers/media/i2c/ov02a10.c | 1025 +++++++++++++++++++++++++++++++++++++++++++
> > > 4 files changed, 1040 insertions(+)
> > > create mode 100644 drivers/media/i2c/ov02a10.c
> > >
> >
> > [snip]
> >
> > > +static int ov02a10_probe(struct i2c_client *client)
> > > +{
> > > + struct device *dev = &client->dev;
> > > + struct ov02a10 *ov02a10;
> > > + unsigned int rotation;
> > > + unsigned int clock_lane_tx_speed;
> > > + unsigned int i;
> > > + int ret;
> > > +
> > > + ov02a10 = devm_kzalloc(dev, sizeof(*ov02a10), GFP_KERNEL);
> > > + if (!ov02a10)
> > > + return -ENOMEM;
> > > +
> > > + ret = ov02a10_check_hwcfg(dev, ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to check HW configuration: %d", ret);
> > > + return ret;
> > > + }
> > > +
> > > + v4l2_i2c_subdev_init(&ov02a10->subdev, client, &ov02a10_subdev_ops);
> > > + ov02a10->mipi_clock_tx_speed = OV02A10_MIPI_TX_SPEED_DEFAULT;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SBGGR10_1X10;
> > > +
> > > + /* Optional indication of physical rotation of sensor */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "rotation", &rotation);
> > > + if (!ret && rotation == 180) {
> > > + ov02a10->upside_down = true;
> > > + ov02a10->fmt.code = MEDIA_BUS_FMT_SRGGB10_1X10;
> > > + }
> > > +
> > > + /* Optional indication of mipi TX speed */
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "ovti,mipi-tx-speed",
> > > + &clock_lane_tx_speed);
> > > +
> > > + if (!ret)
> > > + ov02a10->mipi_clock_tx_speed = clock_lane_tx_speed;
> > > +
> > > + /* Get system clock (eclk) */
> > > + ov02a10->eclk = devm_clk_get(dev, "eclk");
> > > + if (IS_ERR(ov02a10->eclk)) {
> > > + ret = PTR_ERR(ov02a10->eclk);
> > > + dev_err(dev, "failed to get eclk %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency",
> > > + &ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get eclk frequency\n");
> > > + return ret;
> > > + }
> > > +
> > > + ret = clk_set_rate(ov02a10->eclk, ov02a10->eclk_freq);
> > > + if (ret) {
> > > + dev_err(dev, "failed to set eclk frequency (24MHz)\n");
> > > + return ret;
> > > + }
> > > +
> > > + if (clk_get_rate(ov02a10->eclk) != OV02A10_ECLK_FREQ) {
> > > + dev_warn(dev, "wrong eclk frequency %d Hz, expected: %d Hz\n",
> > > + ov02a10->eclk_freq, OV02A10_ECLK_FREQ);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ov02a10->pd_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
> > > + if (IS_ERR(ov02a10->pd_gpio)) {
> > > + ret = PTR_ERR(ov02a10->pd_gpio);
> > > + dev_err(dev, "failed to get powerdown-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ov02a10->n_rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > > + if (IS_ERR(ov02a10->n_rst_gpio)) {
> > > + ret = PTR_ERR(ov02a10->n_rst_gpio);
> > > + dev_err(dev, "failed to get reset-gpios %d\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(ov02a10_supply_names); i++)
> > > + ov02a10->supplies[i].supply = ov02a10_supply_names[i];
> > > +
> > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov02a10_supply_names),
> > > + ov02a10->supplies);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get regulators\n");
> > > + return ret;
> > > + }
> > > +
> > > + mutex_init(&ov02a10->mutex);
> > > + ov02a10->cur_mode = &supported_modes[0];
> > > + ret = ov02a10_initialize_controls(ov02a10);
> > > + if (ret) {
> > > + dev_err(dev, "failed to initialize controls\n");
> > > + goto err_destroy_mutex;
> > > + }
> > > +
> > > + ov02a10->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> > > + ov02a10->subdev.entity.ops = &ov02a10_subdev_entity_ops;
> > > + ov02a10->subdev.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > > + ov02a10->pad.flags = MEDIA_PAD_FL_SOURCE;
> > > + ret = media_entity_pads_init(&ov02a10->subdev.entity, 1, &ov02a10->pad);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to init entity pads: %d", ret);
> > > + goto err_free_handler;
> > > + }
> > > +
> > > + pm_runtime_enable(dev);
> > > + if (!pm_runtime_enabled(dev)) {
> > > + ret = ov02a10_power_on(dev);
> > > + if (ret < 0) {
> > > + dev_err(dev, "failed to power on: %d\n", ret);
> > > + goto err_free_handler;
This is actually wrong, which should be replaced of "err_clean_entity".
> > > + }
> > > + }
> > > +
> > > + ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > > + if (ret) {
> > > + dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > > + if (!pm_runtime_enabled(dev))
> > > + ov02a10_power_off(dev);
>
> This should be moved to error handling section below.
>
Fine.
It would be abstracted as "err_async_register" in next release.
Something like:
err_async_register:
if (!pm_runtime_enabled(dev))
ov02a10_power_off(dev);
err_clean_entity:
media_entity_cleanup(&ov02a10->subdev.entity);
...
> > > + goto err_clean_entity;
> > > + }
> >
> > Tomasz, Sakari, is this ok?
> > or coding like this:
> >
> > ret = v4l2_async_register_subdev(&ov02a10->subdev);
> > if (!pm_runtime_enabled(dev))
> > ov02a10_power_off(dev);
> > if (ret) {
> > dev_err(dev, "failed to register V4L2 subdev: %d", ret);
> > goto err_clean_entity;
> > }
> >
> > What's your opinions about the change?
>
> This turns power off if runtime PM is disabled. I'd keep it as-is, as it'd
> require re-implementing what runtime PM is used for now --- and that's not
> a sensor driver's job.
>
> >
> > > +
> > > + return 0;
> > > +
> > > +err_clean_entity:
> > > + media_entity_cleanup(&ov02a10->subdev.entity);
> > > +err_free_handler:
> > > + v4l2_ctrl_handler_free(ov02a10->subdev.ctrl_handler);
> > > +err_destroy_mutex:
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int ov02a10_remove(struct i2c_client *client)
> > > +{
> > > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > > + struct ov02a10 *ov02a10 = to_ov02a10(sd);
> > > +
> > > + v4l2_async_unregister_subdev(sd);
> > > + media_entity_cleanup(&sd->entity);
> > > + v4l2_ctrl_handler_free(sd->ctrl_handler);
> > > + pm_runtime_disable(&client->dev);
> > > + if (!pm_runtime_status_suspended(&client->dev))
> > > + ov02a10_power_off(&client->dev);
> > > + pm_runtime_set_suspended(&client->dev);
> > > + mutex_destroy(&ov02a10->mutex);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct of_device_id ov02a10_of_match[] = {
> > > + { .compatible = "ovti,ov02a10" },
> > > + {}
> > > +};
> > > +MODULE_DEVICE_TABLE(of, ov02a10_of_match);
> > > +
> > > +static struct i2c_driver ov02a10_i2c_driver = {
> > > + .driver = {
> > > + .name = "ov02a10",
> > > + .pm = &ov02a10_pm_ops,
> > > + .of_match_table = ov02a10_of_match,
> > > + },
> > > + .probe_new = &ov02a10_probe,
> > > + .remove = &ov02a10_remove,
> > > +};
> > > +
> > > +module_i2c_driver(ov02a10_i2c_driver);
> > > +
> > > +MODULE_AUTHOR("Dongchun Zhu <dongchun.zhu@mediatek.com>");
> > > +MODULE_DESCRIPTION("OmniVision OV02A10 sensor driver");
> > > +MODULE_LICENSE("GPL v2");
> > > +
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V6, 2/2] media: i2c: dw9768: Add DW9768 VCM driver
From: Dongchun Zhu @ 2020-06-05 3:28 UTC (permalink / raw)
To: Sakari Ailus
Cc: Mark Rutland, Nicolas Boichat, Andy Shevchenko, srv_heupstream,
linux-devicetree, Linus Walleij,
Shengnan Wang (王圣男), Tomasz Figa,
Bartosz Golaszewski, Sj Huang, Rob Herring,
moderated list:ARM/Mediatek SoC support, dongchun.zhu, Louis Kuo,
Matthias Brugger, Cao Bing Bu, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
In-Reply-To: <20200604081032.GG16711@paasikivi.fi.intel.com>
Hi Sakari,
On Thu, 2020-06-04 at 11:10 +0300, Sakari Ailus wrote:
> On Thu, Jun 04, 2020 at 10:33:38AM +0800, Dongchun Zhu wrote:
> > Hi Tomasz,
> >
> > On Mon, 2020-06-01 at 20:47 +0200, Tomasz Figa wrote:
> > > On Wed, May 27, 2020 at 11:03 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Mon, 2020-05-25 at 13:45 +0200, Tomasz Figa wrote:
> > > > > On Fri, May 22, 2020 at 11:27 AM Dongchun Zhu <dongchun.zhu@mediatek.com> wrote:
> > > > > >
> > > > > > Hi Tomasz,
> > > > > >
> > > > > > Thanks for the review. My replies are as below.
> > > > > >
> > > > > > On Thu, 2020-05-21 at 19:51 +0000, Tomasz Figa wrote:
> > > > > > > Hi Dongchun, Sakari,
> > > > > > >
> > > > > > > On Mon, May 18, 2020 at 09:27:31PM +0800, Dongchun Zhu wrote:
> > > > > [snip]
> > > > > > > > + pm_runtime_enable(dev);
> > > > > > > > + if (!pm_runtime_enabled(dev)) {
> > > > > > > > + ret = dw9768_runtime_resume(dev);
> > > > > > > > + if (ret < 0) {
> > > > > > > > + dev_err(dev, "failed to power on: %d\n", ret);
> > > > > > > > + goto entity_cleanup;
> > > > > > > > + }
> > > > > > > > + }
> > > > > > > > +
> > > > > > > > + ret = v4l2_async_register_subdev(&dw9768->sd);
> > > > > > > > + if (ret < 0)
> > > > > > > > + goto entity_cleanup;
> > > > > > > > +
> > > > > > > > + return 0;
> > > > > > > > +
> > > > > > > > +entity_cleanup:
> > > > > > >
> > > > > > > Need to power off if the code above powered on.
> > > > > > >
> > > > > >
> > > > > > Thanks for the reminder.
> > > > > > If there is something wrong with runtime PM, actuator is to be powered
> > > > > > on via dw9768_runtime_resume() API.
> > > > > > When actuator sub-device is powered on completely and async registered
> > > > > > successfully, we shall power off it afterwards.
> > > > > >
> > > > >
> > > > > The code above calls dw9768_runtime_resume() if
> > > > > !pm_runtime_enabled(dev), but the clean-up code below the
> > > > > entity_cleanup label doesn't have the corresponding
> > > > > dw9768_runtime_suspend() call.
> > > > >
> > > >
> > > > Did you mean the 'entity_cleanup' after v4l2_async_register_subdev()?
> > >
> > > Yes.
> > >
> > > > Actually I made some changes for OV02A V9, according to this comment.
> > > > Could you help review that change? Thanks.
> > >
> > > Sure, I will take a look.
> > >
> >
> > Thanks.
> > Sorry, I just wanna make sure the change is okay for next release.
> > May we use the check like OV02A V9 did?
> > ret = v4l2_async_register_subdev(&dw9768->sd);
> > if (ret < 0) {
> > if (!pm_runtime_enabled(dev))
> > dw9768_runtime_suspend(dev);
>
> Please do this part where you're jumping to, if possible.
>
Fine.
Fixed in next release.
> > goto entity_cleanup;
> > }
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 4/4] ARM: DT changes for v5.8
From: pr-tracker-bot @ 2020-06-05 3:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: SoC Team, Linus Torvalds, Linux Kernel Mailing List, Linux ARM
In-Reply-To: <CAK8P3a2WM_iT+zPZjDeoih-qZwSJbjZfVMvqpG3Sa1aDqgHwPA@mail.gmail.com>
The pull request you sent on Thu, 4 Jun 2020 22:56:12 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git tags/arm-dt-5.8
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/9d71d3cd9ef040c284506648285915e9ba4d08c4
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 3/4] ARM: driver updates for v5.8
From: pr-tracker-bot @ 2020-06-05 3:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: SoC Team, Linus Torvalds, Linux Kernel Mailing List, Linux ARM
In-Reply-To: <CAK8P3a2fCyYgoexi6NiAY2cch5C-7EEkNGy6PJGxjJ-2yMndLA@mail.gmail.com>
The pull request you sent on Thu, 4 Jun 2020 22:54:31 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git tags/arm-drivers-5.8
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/828f3e18e1cb98c68fc6db4d5113513d4a267775
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 2/4]ARM: defconfig updates for v5.8
From: pr-tracker-bot @ 2020-06-05 3:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: SoC Team, Linus Torvalds, Linux Kernel Mailing List, Linux ARM
In-Reply-To: <CAK8P3a3w4euZfDQPt7wqWg9w4uf7SM4NLeA2CyOMmgNGPAdQaQ@mail.gmail.com>
The pull request you sent on Thu, 4 Jun 2020 22:52:48 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git tags/arm-defconfig-5.8
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/298743c193bb50b5d65b9285eb7206ffb31d412d
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [GIT PULL 1/4] ARM: SoC changes for v5.8
From: pr-tracker-bot @ 2020-06-05 3:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: SoC Team, Linus Torvalds, Linux Kernel Mailing List, Linux ARM
In-Reply-To: <CAK8P3a00L4n3b=X+PQXe1pxf9CHryZTes9L1MD5i2+0RLXprfw@mail.gmail.com>
The pull request you sent on Thu, 4 Jun 2020 22:50:34 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git tags/arm-soc-5.8
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/694b5a5d313f3997764b67d52bab66ec7e59e714
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH v8 00/13] add ecspi ERR009165 for i.mx6/7 soc family
From: Robin Gong @ 2020-06-05 2:45 UTC (permalink / raw)
To: Matthias Schiffer
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
festevam@gmail.com, martin.fuzzey@flowbird.group, Markus Niebel,
catalin.marinas@arm.com, s.hauer@pengutronix.de,
will.deacon@arm.com, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, linux-spi@vger.kernel.org, vkoul@kernel.org,
broonie@kernel.org, dl-linux-imx, kernel@pengutronix.de,
u.kleine-koenig@pengutronix.de, dan.j.williams@intel.com,
shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <7cb6cf02fc7cf860d1da7ba889887e79623e71a9.camel@ew.tq-group.com>
[-- Attachment #1: Type: text/plain, Size: 4271 bytes --]
On 2020/06/03 Matthias Schiffer <matthias.schiffer@ew.tq-group.com> wrote:
> On Wed, 2020-06-03 at 09:50 +0000, Robin Gong wrote:
> > On 2020/06/03 Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > wrote:
> > > On Thu, 2020-05-21 at 04:34 +0800, Robin Gong wrote:
> > > > There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO
> > > > transfer to be send twice in DMA mode. Please get more information
> > > > from:
> > > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> > > > .
> > > >
> > >
> > >
> nxp.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7C
> > > yibin.g
> > > >
> > >
> > >
> ong%40nxp.com%7C4621358b9be04a79d2d508d80798835b%7C686ea1d3bc2b
> > > 4c6fa92
> > > >
> > >
> > >
> cd99c5c301635%7C0%7C1%7C637267698912634476&sdata=hR66H1hP%
> > > 2Fqb6OXe
> > > > w9wpXizY8DiNfZZ1KLwu3Kty87jc%3D&reserved=0. The workaround
> is
> > > > adding new sdma ram script which works in XCH mode as PIO inside
> > > > sdma instead of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0.
> > > > The issue
> > >
> > > should be exist on all legacy i.mx6/7 soc family before i.mx6ul.
> > > > NXP fix this design issue from i.mx6ul, so newer chips including
> > > > i.mx6ul/ 6ull/6sll do not need this workaroud anymore. All other
> > > > i.mx6/7/8 chips still need this workaroud. This patch set add new
> > > > 'fsl,imx6ul-ecspi'
> > > > for ecspi driver and 'ecspi_fixed' in sdma driver to choose if
> > > > need errata or not.
> > > > The first two reverted patches should be the same issue, though,
> > > > it seems 'fixed' by changing to other shp script. Hope Sean or
> > > > Sascha could have the chance to test this patch set if could fix
> > > > their issues.
> > > > Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not
> > > > work on i.mx8mm because the event id is zero.
> > > >
> > > > PS:
> > > > Please get sdma firmware from below linux-firmware and copy it
> > > > to your local rootfs /lib/firmware/imx/sdma.
> > >
> > >
> > > Hello Robin,
> > >
> > > we have tried out this series, and there seems to be an issue with
> > > the
> > > PIO fallback. We are testing on an i.MX6Q board, and our kernel is
> > > a
> > > mostly-unmodified 5.4, on which we backported all SDMA patches from
> > > next-20200602 (imx-sdma.c is identical to next-20200602 version),
> > > and
> > > then applied this whole series.
> > >
> > > We build the SDMA driver as a kernel module, which is loaded by
> > > udev,
> > > so the root filesystem is ready and the SDMA firmware can be
> > > loaded.
> > > The behaviour we're seeing is the following:
> > >
> > > 1. As long as the SDMA driver is not loaded, initializing spi_imx
> > > will
> > > be deferred
> > > 2. imx_sdma is loaded. The SDMA firmware is not yet loaded at this
> > > point
> > > 3. spi_imx is initialized and an SPI-NOR flash is probed. To load
> > > the
> > > BFPT, the driver will attempt to use DMA; this will fail with
> > > EINVAL as
> > > long as the SDMA firmware is not ready, so the fallback to PIO
> > > happens
> > > (4. SDMA firmware is ready, subsequent SPI transfers use DMA)
> > >
> > > The problem happens in step 3: Whenever the driver falls back to
> > > PIO,
> > > the received data is corrupt. The behaviour is specific to the
> > > fallback: When I disable DMA completely via spi_imx.use_dma, or
> > > when
> > > the timing is lucky and the SDMA firmware gets loaded before the
> > > flash
> > > is probed, no corruption can be observed.
> >
> > Thanks Matthias, would you like post log?
> >
>
> I have attached the following log files:
> - pio.log: DMA disabled via module parameter
> - dma.log: "lucky" timing, SDMA firmware loaded before SPI-NOR probe
> - fallback.log: DMA->PIO fallback
>
> The logs include some additional log messages:
> - Return value of spi_imx_dma_transfer() before PIO fallback
> - SPI-NOR SFPT dump
>
> It can be seen that the BFPT data is identical in pio.log and dma.log,
> and differs almost completely in fallback.log. The corrupted data seems
> to be random, or uninitialized memory; it differs with every boot.
Would you please have a try with the attached patch? Thanks.
[-- Attachment #2: 0006-spi-imx-add-dma_sync_sg_for_device-after-fallback-fr.patch --]
[-- Type: application/octet-stream, Size: 1526 bytes --]
From 08becec165b15663fafea52e3dc6ed5482ad3652 Mon Sep 17 00:00:00 2001
From: Robin Gong <yibin.gong@nxp.com>
Date: Fri, 5 Jun 2020 08:57:19 +0800
Subject: [PATCH v9 06/14] spi: imx: add dma_sync_sg_for_device after fallback
from dma
In case dma transfer failed and fallback to pio, tx_buf/rx_buf need to be
taken care cache since they have already been mapped by dma_map_sg() in
spi.c.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/spi/spi-imx.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index b7a85e3..c51cd3a 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1456,6 +1456,13 @@ static int spi_imx_pio_transfer(struct spi_device *spi,
return -ETIMEDOUT;
}
+ if (transfer->rx_sg.sgl) {
+ struct device *rx_dev = spi->controller->dma_rx->device->dev;
+
+ dma_sync_sg_for_device(rx_dev, transfer->rx_sg.sgl,
+ transfer->rx_sg.nents, DMA_TO_DEVICE);
+ }
+
return transfer->len;
}
@@ -1521,10 +1528,15 @@ static int spi_imx_transfer(struct spi_device *spi,
* firmware may not be updated as ERR009165 required.
*/
if (spi_imx->usedma) {
+ struct device *tx_dev = spi->controller->dma_tx->device->dev;
+
ret = spi_imx_dma_transfer(spi_imx, transfer);
if (ret != -EINVAL)
return ret;
+ dma_sync_sg_for_device(tx_dev, transfer->tx_sg.sgl,
+ transfer->tx_sg.nents, DMA_FROM_DEVICE);
+
spi_imx->devtype_data->disable_dma(spi_imx);
spi_imx->usedma = false;
--
2.7.4
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v3 2/6] PCI: uniphier: Add misc interrupt handler to invoke PME and AER
From: Kunihiko Hayashi @ 2020-06-05 2:36 UTC (permalink / raw)
To: Marc Zyngier
Cc: devicetree, Lorenzo Pieralisi, Masami Hiramatsu, Jassi Brar,
Jingoo Han, linux-pci, linux-kernel, Masahiro Yamada, Rob Herring,
Gustavo Pimentel, Bjorn Helgaas, linux-arm-kernel
In-Reply-To: <9cbfdacba32c5e351fd9e14444768666@kernel.org>
Hi Marc,
On 2020/06/04 19:11, Marc Zyngier wrote:
> On 2020-06-04 10:43, Kunihiko Hayashi wrote:
>
> [...]
>
>>>> -static void uniphier_pcie_irq_handler(struct irq_desc *desc)
>>>> +static void uniphier_pcie_misc_isr(struct pcie_port *pp)
>>>> {
>>>> - struct pcie_port *pp = irq_desc_get_handler_data(desc);
>>>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>>> struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
>>>> - struct irq_chip *chip = irq_desc_get_chip(desc);
>>>> - unsigned long reg;
>>>> - u32 val, bit, virq;
>>>> + u32 val, virq;
>>>>
>>>> - /* INT for debug */
>>>> val = readl(priv->base + PCL_RCV_INT);
>>>>
>>>> if (val & PCL_CFG_BW_MGT_STATUS)
>>>> dev_dbg(pci->dev, "Link Bandwidth Management Event\n");
>>>> +
>>>> if (val & PCL_CFG_LINK_AUTO_BW_STATUS)
>>>> dev_dbg(pci->dev, "Link Autonomous Bandwidth Event\n");
>>>> - if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS)
>>>> - dev_dbg(pci->dev, "Root Error\n");
>>>> - if (val & PCL_CFG_PME_MSI_STATUS)
>>>> - dev_dbg(pci->dev, "PME Interrupt\n");
>>>> +
>>>> + if (pci_msi_enabled()) {
>>>
>>> This checks whether the kernel supports MSIs. Not that they are
>>> enabled in your controller. Is that really what you want to do?
>>
>> The below two status bits are valid when the interrupt for MSI is asserted.
>> That is, pci_msi_enabled() is wrong.
>>
>> I'll modify the function to check the two bits only if this function is
>> called from MSI handler.
>>
>>>
>>>> + if (val & PCL_CFG_AER_RC_ERR_MSI_STATUS) {
>>>> + dev_dbg(pci->dev, "Root Error Status\n");
>>>> + virq = irq_linear_revmap(pp->irq_domain, 0);
>>>> + generic_handle_irq(virq);
>>>> + }
>>>> +
>>>> + if (val & PCL_CFG_PME_MSI_STATUS) {
>>>> + dev_dbg(pci->dev, "PME Interrupt\n");
>>>> + virq = irq_linear_revmap(pp->irq_domain, 0);
>>>> + generic_handle_irq(virq);
>>>> + }
>>>
>>> These two cases do the exact same thing, calling the same interrupt.
>>> What is the point of dealing with them independently?
>>
>> Both PME and AER are asserted from MSI-0, and each handler checks its own
>> status bit in the PCIe register (aer_irq() in pcie/aer.c and pcie_pme_irq()
>> in pcie/pme.c).
>> So I think this handler calls generic_handle_irq() for the same MSI-0.
>
> So what is wrong with
>
> if (val & (PCL_CFG_AER_RC_ERR_MSI_STATUS |
> PCL_CFG_PME_MSI_STATUS)) {
> // handle interrupt
> }
>
> ?
No problem.
I'll rewrite it in the same way as yours in handling interrupts.
> If you have two handlers for the same interrupt, this is a shared
> interrupt and each handler will be called in turn.
Yes, MSI-0 is shared with PME and AER, and it will be like that.
Thank you,
---
Best Regards
Kunihiko Hayashi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox