Message-ID: <7c84cbd1808dc89f9ddaece13f846c4fbd12935a.camel@nxp.com>
Subject: Re: [PATCH] driver: bus: simple-pm-bus: Add Freescale i.MX8qm/qxp
 CSR compatible strings
From:   Liu Ying <victor.liu@nxp.com>
To:     Geert Uytterhoeven <geert@linux-m68k.org>
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
Date:   Thu, 26 Jan 2023 10:54:54 +0800
In-Reply-To: <CAMuHMdU0KaRyP0qxJLwit1G=7zMr1k5wpCkG-wD31958mrKRWg@mail.gmail.com>
References: <20230125083248.1305270-1-victor.liu@nxp.com>
         <CAMuHMdU0KaRyP0qxJLwit1G=7zMr1k5wpCkG-wD31958mrKRWg@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZUo3TWw2dnU4VTdZSG0rSWtwbnpFM2h0RG5CVkRocFo1MWtKWXA4bzJiRk1x?=
 =?utf-8?B?S3dobWJqYTZCTzl5aS9UQmdteFAzcTQyUHpVTkdmWDJTZ3RYa0cvcTR4UVVW?=
 =?utf-8?B?Vno3NTk2NDJ4c04vWjBveTFxLzJJd3hqQlVEMGhCenRCbWlSQStPV3Z5bmx2?=
 =?utf-8?B?a0ZqZkpVd28vTlRtKyt4WjlNcU0wajVuRjVwZUJIRHcwZ0RDMWN6dzRlY1Ft?=
 =?utf-8?B?SlViSFdYaUtKbGpFM01kRTE4bDJPRXJlb0pUdlpjeXBXUXFJRndUQVJpQllB?=
 =?utf-8?B?VXpnMmZQbkNCb3BCSWVBaGJ6bHVRUndpOUJkRm9DblpXWmFRMDgzVHBiWTNw?=
 =?utf-8?B?NkFKQ0ZqVzJmc3EzUUtXeWgyTTVFY2xoSjU3SzhodWRtYWtGU1NTa0FlUjZY?=
 =?utf-8?B?QzNtSk9YK1VGZlJoM0NNbEV5Sko5N29YMkpKQUhhejNGclY5Y0FUTHp5SVha?=
 =?utf-8?B?Sm1Od1BxeldzdXpEUnBna3VseXh6UndrSlRRQTRRVjhHaEtPeWZzSkRFanRM?=
 =?utf-8?B?ZWZOOUJJRExYOGQ1ZXZLRXZVV1JQN3dkOE9lWUVPWTRlMlhtTk1MdG5tMGJC?=
 =?utf-8?B?Ny9ucGtnN0ZzU0xRMTVxVGZpUkIwZjFnamwvektwL0gvdnIvS01EVnF3bm84?=
 =?utf-8?B?RHhhdHVBdXhlSTFnKy9zdkhaZEVpMC96MkczMkhqNTVVQXkwa2FUckV4S21o?=
 =?utf-8?B?TG54OXdzcks1WHBTWjl2MVpuRW11MWlzVTNqNkNQSmMwMTVFaDJCdTNPVHND?=
 =?utf-8?B?U1BGT2lDakkwT2xGU0o4M1FHMmFTMmJoYWdWVkpRRno5VmhNOGw0WmxsYVJJ?=
 =?utf-8?B?ZHRncSt3U0xRM0VseXZJVVI5b3NvNVJCcDZyR2lwdDZJdDhzV3pNcXcxcSs5?=
 =?utf-8?B?TGpsV1RPQ2tlMXkxVmFqSTh0MVhYQ1dBV0luNUlSL2VFcWczc2xuSTczNXZz?=
 =?utf-8?B?dHNOY045d01jNzJZQXB5TjJGZC9kakZOWlN6RXExdGFPWVUvN2F3Wk1VN1hD?=
 =?utf-8?B?WEhsSVhXZG15NzBpZVlNMURMSjkzMEMwTzhNLzdmclNHR3RCN3Vvd1VQTUJC?=
 =?utf-8?B?ck9yaXhVODFKbHdka1FuVUxBbndMbkd1NFhWVUhpL1RqamhuZ2FndjEyQjBw?=
 =?utf-8?B?akkxOE1EUGYraWpmZWY4N0FZMWJEUSthSXlUVVJLQ2tGbUNJWnF1UytEd2Ry?=
 =?utf-8?B?SEwweUFqQi9pby9JM1RpU2V3MldvT0NCb0hWcU1MZm1TU2w2ekQvNktXMGhV?=
 =?utf-8?B?S2hYUnBla1N3NjFIbzlHMkh6RFZzOVVOZEp3ZVZOQ1RITmpRNWU1dlZMSCtv?=
 =?utf-8?B?L3ZuNkNIcWdkSzNkeEJmbVdTTlEwOXg1K1RUOTJ1a3hKa2ZvSmtQM1BVZjhC?=
 =?utf-8?B?cmRhbzdabEJndUpsTlc1T1VJSkNHNmhMN2RBK3NMZFhDNFlFKzdaUTJqK2dr?=
 =?utf-8?B?TjhubkxVNndOK1QvTVEzYUM2OG1PZkF1N1VFK0hKSlJkb3cxYmcvWkxvK0dj?=
 =?utf-8?B?TEdpSW5HUVhhZndYMFN2NjcvU2ZVb2tob0FYTzRLR1lraVhjaUpNK0RhbjBk?=
 =?utf-8?B?S2pycU0zSjJsY0J5US9yL1RTTXJxWnF6aEpMOWd4emdLMlZYZXZqNHBaaXJv?=
 =?utf-8?B?S010aVlseHp2eUxnRllZK290a28yWitFbmozZDFlNzVwejhxQ3p0RFpDWXR4?=
 =?utf-8?B?K2gxenhlUFBxaWhjSm5GLzFWaU1CYm9lQmpLQ3FHdXdTNFY4YmYxc3dhYlFY?=
 =?utf-8?B?cEY5YVh6Y2kzVzZLZnRjRHRqRmt5YXRSeVFiQXVnV1RGZFNWTm9UN3lxSlBv?=
 =?utf-8?B?TzdoSmJORHZlMVVSTWI3bEViL2hNcm9iR0FnZ0VXb0hKZVYwZVI4TjJQMXpZ?=
 =?utf-8?B?Y0JaRzdiVlIvVmZ4eFZLS0wySzBGSjI1K3NnY0Q3bHlqaHZmNzc1MmZLdlB1?=
 =?utf-8?B?K3BoWUh0SmUzdlg1dk8rejYwcDJmRHJHVENSSXNmTzVyYmJMY0kzLzVvZkJJ?=
 =?utf-8?B?YkxZNnFpa214dUdjWExnbFVVVnJDYU9yZWZCL2UxM0dXV0ZqVnR3UnRUMG9q?=
 =?utf-8?B?cFpxVUxGbWpYNDBBeWFLTWpQUzVGNC9DVXFsS3dxY1RZWDd5L3VSUHJEWlVP?=
 =?utf-8?Q?XJ05ArFyxA2wJ+yUc8n78v+CK?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ddd50165-5bc3-491b-9469-08daff48cf6f
X-MS-Exchange-CrossTenant-AuthSource: AM7PR04MB7046.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jan 2023 02:55:41.2408
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 37Th3KOQs5qnhiITsIr9DBDkbReAyE6OIBAXxjFPzXK792ChL6CTrabpHlvgomSO3jCYz/iPRlqF4D+cKjxepg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU2PR04MB8710
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, 2023-01-25 at 10:05 +0100, Geert Uytterhoeven wrote:
> Hi Liu,

Hi Geert,

> 
> 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
"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...

Hmm, fsl,imx8qxp-csr.yaml was first written earlier than the below
patch.  Without that patch, child devices of the CSR module can be
populated.

98e96cf80045 ("drivers: bus: simple-pm-bus: Add support for probing
simple bus only devices")

Should I go ahead to fix fsl,imx8qxp-csr.yaml and move it to bus
directory?

Regards,
Liu Ying

> 
> >         { /* sentinel */ }
> >  };
> >  MODULE_DEVICE_TABLE(of, simple_pm_bus_of_match);
> 
> 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