From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Liu Ying <victor.liu@nxp.com>
Cc: linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org,
linux-imx@nxp.com, Rob Herring <robh@kernel.org>,
Lee Jones <lee@kernel.org>,
krzysztof.kozlowski+dt@linaro.org, robh+dt@kernel.org
Subject: Re: [PATCH] driver: bus: simple-pm-bus: Add Freescale i.MX8qm/qxp CSR compatible strings
Date: Thu, 26 Jan 2023 09:30:50 +0100 [thread overview]
Message-ID: <CAMuHMdWCBNctQq8Bg0DQFC-Ap_JRzoMtAFozfaB1OssE7Lr6Bg@mail.gmail.com> (raw)
In-Reply-To: <7c84cbd1808dc89f9ddaece13f846c4fbd12935a.camel@nxp.com>
Hi Liu,
On Thu, Jan 26, 2023 at 3:55 AM Liu Ying <victor.liu@nxp.com> wrote:
> On Wed, 2023-01-25 at 10:05 +0100, Geert Uytterhoeven wrote:
> > On Wed, Jan 25, 2023 at 9:31 AM Liu Ying <victor.liu@nxp.com> wrote:
> > > Freescale i.MX8qm/qxp CSR module matches with what the simple power
> > > managed bus driver does, considering it needs an IPG clock to be
> > > enabled before accessing it's child devices, the child devices need
> > > to be populated by the CSR module and the child devices' power
> > > management operations need to be propagated to their parent
> > > devices.
> > > Add the CSR module's compatible strings to simple_pm_bus_of_match[]
> > > table to support the CSR module.
> > >
> > > Suggested-by: Rob Herring <robh@kernel.org>
> > > Suggested-by: Lee Jones <lee@kernel.org>
> > > Signed-off-by: Liu Ying <victor.liu@nxp.com>
> >
> > Thanks for your patch!
>
> Thanks for your review!
>
> >
> > > ---
> > > The CSR module's dt-binding documentation can be found at
> > > Documentation/devicetree/bindings/mfd/fsl,imx8qxp-csr.yaml.
> > >
> > > Suggested by Rob and Lee in this thread:
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fproject%2Flinux-arm-kernel%2Fpatch%2F20221017075702.4182846-1-victor.liu%40nxp.com%2F&data=05%7C01%7Cvictor.liu%40nxp.com%7C3fa98ff270534078019508dafeb34b10%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638102343312884116%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zm8Z5gWt9yGXakYlxopUfZKLMUJRWTxq1kWHLyqhyww%3D&reserved=0
> > >
> > > drivers/bus/simple-pm-bus.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-
> > > bus.c
> > > index 7afe1947e1c0..4a7575afe6c6 100644
> > > --- a/drivers/bus/simple-pm-bus.c
> > > +++ b/drivers/bus/simple-pm-bus.c
> > > @@ -120,6 +120,8 @@ static const struct of_device_id
> > > simple_pm_bus_of_match[] = {
> > > { .compatible = "simple-mfd", .data = ONLY_BUS },
> > > { .compatible = "isa", .data = ONLY_BUS },
> > > { .compatible = "arm,amba-bus", .data = ONLY_BUS },
> > > + { .compatible = "fsl,imx8qm-lvds-csr", },
> > > + { .compatible = "fsl,imx8qxp-mipi-lvds-csr", },
> >
> > I did read the thread linked above, and I still think you should just
> > add "simple-pm-bus" to the compatible value in DTS, so no driver
> > change
> > is needed, cfr.
> > Documentation/devicetree/bindings/bus/renesas,bsc.yaml.
>
> This means that i.MX8qm/qxp CSR module dt-binding documentation needs
> to be changed. I'd like to know how Rob and Krzysztof think about
> that.
>
> >
> > If that doesn't work due to DT binding constraints, the latter should
> > be fixed.
>
> Adding "simple-pm-bus" to the compatible value in DTS doesn't work,
> because "simple-mfd" is matched first and "ONLY_BUS" is set for
Is that because you have both "simple-mfd" and "simple-pm-bus",
and the former is listed first in DTS?
Does it work if you change the order?
> "simple-mfd". s/simple-mfd/simple-pm-bus/ for the compatbile value in
> DTS does work, but it means that fsl,imx8qxp-csr.yaml needs to be
> changed and moved from mfd directory to bus directory...
BTW, originally I didn't want to introduce "simple-pm-bus" at all,
and make it just call pm_runtime_enable() for "simple-bus" (PM is
everywhere anyway), but that was rejected...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
next prev parent reply other threads:[~2023-01-26 8:31 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-25 8:32 [PATCH] driver: bus: simple-pm-bus: Add Freescale i.MX8qm/qxp CSR compatible strings Liu Ying
2023-01-25 9:05 ` Geert Uytterhoeven
2023-01-26 2:54 ` Liu Ying
2023-01-26 8:30 ` Geert Uytterhoeven [this message]
2023-01-26 9:06 ` Liu Ying
2023-01-26 12:45 ` Krzysztof Kozlowski
2023-01-29 8:13 ` Liu Ying
2023-01-29 10:49 ` Krzysztof Kozlowski
2023-01-30 1:45 ` Liu Ying
2023-02-01 7:31 ` Krzysztof Kozlowski
2023-02-01 7:40 ` Liu Ying
2023-08-22 8:16 ` Ying Liu
2023-01-30 8:13 ` Geert Uytterhoeven
2023-01-31 5:26 ` Liu Ying
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=CAMuHMdWCBNctQq8Bg0DQFC-Ap_JRzoMtAFozfaB1OssE7Lr6Bg@mail.gmail.com \
--to=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=lee@kernel.org \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=robh@kernel.org \
--cc=victor.liu@nxp.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).