From: Patrick DELAUNAY <patrick.delaunay@foss.st.com>
To: Conor Dooley <conor@kernel.org>
Cc: Marek Vasut <marex@denx.de>,
Alexandre TORGUE <alexandre.torgue@foss.st.com>,
Liam Girdwood <lgirdwood@gmail.com>,
Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Pascal Paillet <p.paillet@foss.st.com>,
<devicetree@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
<linux-kernel@vger.kernel.org>,
<linux-stm32@st-md-mailman.stormreply.com>
Subject: Re: [PATCH v3 1/2] dt-bindings: regulator: st,stm32mp1-pwr-reg: add compatible for STM32MP13
Date: Wed, 15 May 2024 16:33:22 +0200 [thread overview]
Message-ID: <0c97408c-422d-46b3-8017-da9ebb0767e1@foss.st.com> (raw)
In-Reply-To: <20240514-entryway-idealize-fcd5ed0e1de7@spud>
Hi,
On 5/14/24 22:08, Conor Dooley wrote:
> On Mon, May 13, 2024 at 08:02:21PM +0200, Patrick DELAUNAY wrote:
>> Hi,
>>
>> On 5/13/24 17:16, Conor Dooley wrote:
>>> On Mon, May 13, 2024 at 04:34:20PM +0200, Marek Vasut wrote:
>>>> On 5/13/24 11:56 AM, Patrick Delaunay wrote:
>>>>> Add new compatible "st,stm32mp13-pwr-reg" for STM32MP13 SoC family.
>>>>>
>>>>> Signed-off-by: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>>>> ---
>>>>>
>>>>> Changes in v3:
>>>>> - Replace oneOf/const by enum; solve the V2 issues for dt_binding_check
>>>>>
>>>>> Changes in v2:
>>>>> - update for Rob review, only add compatible for STM32MP13 family
>>>>>
>>>>> .../devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml | 4 +++-
>>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
>>>>> index c9586d277f41..c766f0a15a31 100644
>>>>> --- a/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
>>>>> +++ b/Documentation/devicetree/bindings/regulator/st,stm32mp1-pwr-reg.yaml
>>>>> @@ -11,7 +11,9 @@ maintainers:
>>>>> properties:
>>>>> compatible:
>>>>> - const: st,stm32mp1,pwr-reg
>>>>> + enum:
>>>>> + - st,stm32mp1,pwr-reg
>>>>> + - st,stm32mp13-pwr-reg
>>>> Should the st,stm32mp1,pwr-reg be treated as fallback compatible for
>>>> st,stm32mp13-pwr-reg or not ?
>>>>
>>>> In other words, should the DT contain:
>>>> compatible = "st,stm32mp13-pwr-reg", "st,stm32mp1,pwr-reg";
>>>> or
>>>> compatible = "st,stm32mp13-pwr-reg";
>>>> ? Which one is preferable ?
>>>>
>>>> I think the former one, since the MP13 PWR block could also be operated by
>>>> older MP1(5) PWR block driver(s) without any adverse effects, except the SD
>>>> IO domain configuration won't be available, right ?
>>> Aye, the fallback sounds like what should be being used here, especially
>>> if another user of the DT might not need to implement the extra domain.
>>
>> Yes it is the the only difference but I think that type of fallback is no
>> more recommended for different device and
>>
>> the PWR device on STM32MP13 and on STM32MP15 are different.
>>
>>
>> The other user of the non-secure device tree don't use the yet the PWR
>> driver for STM32MP13,
>>
>> so for me the fallback is not needed for non secure world (Linux/U-Boot).
>>
>>
>> So I prefer to introduce a new compatible in Linux kernel before the
>> STM32MP13 PWR node is really used to avoid ABI break in futur.
> How is it going to break an ABI? If the mp13 implements a functional
> subset of what the mp1 does, then that's what fallback compatibles
> are intended for. Marek's mail suggests that this is the case, and
> therefore a fallback should be used.
>
> If code written for the mp1 would not work on the mp13, then a fallback
> should not be used.
>
> Neither I nor Marek are saying that a new compatible should not be
> created, in case you misunderstood that. This only affects the binding
> patch, and your driver etc are free to use the mp13 compatible.
>
> Thanks,
> Conor.
The IP PWR change with STM32MP13 and it is not just a subset
but for the range used by the compatible <0x50001000 0x10>
I agreed that feature are just added.
Other feature are also removed but for next registers (the associated
registers or bits becomes reserved, for PWR_MCUCR for example).
So I can change in binding to support the DT:
compatible = "st,stm32mp1,pwr-reg";
or for STM32MP13:
compatible = "st,stm32mp13-pwr-reg", "st,stm32mp1,pwr-reg";
with
compatible:
oneOf:
- items:
- const: st,stm32mp1,pwr-reg
- items:
- const: st,stm32mp13-pwr-reg
- const: st,stm32mp1,pwr-reg
But for me this fallback in device tree is a little confusing for PWR.
Patrick
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-05-15 14:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-13 9:56 [PATCH v3 0/2] ARM: st: add new compatible for PWR regulators on STM32MP13 Patrick Delaunay
2024-05-13 9:56 ` [PATCH v3 1/2] dt-bindings: regulator: st,stm32mp1-pwr-reg: add compatible for STM32MP13 Patrick Delaunay
2024-05-13 14:34 ` Marek Vasut
2024-05-13 15:16 ` Conor Dooley
2024-05-13 18:02 ` Patrick DELAUNAY
2024-05-14 20:08 ` Conor Dooley
2024-05-15 14:33 ` Patrick DELAUNAY [this message]
2024-05-15 15:35 ` Conor Dooley
2024-05-15 15:37 ` Marek Vasut
2024-05-15 16:15 ` Conor Dooley
2024-05-16 16:32 ` Patrick DELAUNAY
2024-05-13 9:56 ` [PATCH v3 2/2] regulator: stm32-pwr: add support of STM32MP13 Patrick Delaunay
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=0c97408c-422d-46b3-8017-da9ebb0767e1@foss.st.com \
--to=patrick.delaunay@foss.st.com \
--cc=alexandre.torgue@foss.st.com \
--cc=broonie@kernel.org \
--cc=conor+dt@kernel.org \
--cc=conor@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=marex@denx.de \
--cc=mcoquelin.stm32@gmail.com \
--cc=p.paillet@foss.st.com \
--cc=robh@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: 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).