diff mbox series

[v4,2/3] dt-bindings: pinctrl: aspeed,ast2600-pinctrl

Message ID 20250324175926.222473-3-wthai@nvidia.com (mailing list archive)
State New
Headers show
Series Add device tree for Nvidia's GB200NVL BMC | expand

Commit Message

Willie Thai March 24, 2025, 5:59 p.m. UTC
Add EMMCG5 enum to compatible list of pinctrl binding for emmc enabling.

Cc: Andrew Jeffery <andrew@codeconstruct.com.au>
Signed-off-by: Willie Thai <wthai@nvidia.com>
---
 .../devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml      | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski March 25, 2025, 7:17 a.m. UTC | #1
On 24/03/2025 18:59, Willie Thai wrote:
> Add EMMCG5 enum to compatible list of pinctrl binding for emmc enabling.
> 
> Cc: Andrew Jeffery <andrew@codeconstruct.com.au>
> Signed-off-by: Willie Thai <wthai@nvidia.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Andrew Jeffery March 28, 2025, 12:46 a.m. UTC | #2
On Mon, 2025-03-24 at 17:59 +0000, Willie Thai wrote:
> Add EMMCG5 enum to compatible list of pinctrl binding for emmc
> enabling.
> 
> Cc: Andrew Jeffery <andrew@codeconstruct.com.au>
> Signed-off-by: Willie Thai <wthai@nvidia.com>
> ---
>  .../devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml      | 1
> +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> pinctrl.yaml
> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> pinctrl.yaml
> index 80974c46f3ef..cb75e979f5e0 100644
> --- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> pinctrl.yaml
> @@ -276,6 +276,7 @@ additionalProperties:
>          - BMCINT
>          - EMMCG1
>          - EMMCG4
> +        - EMMCG5

What pin configuration does this correspond to for the eMMC controller?
These groups aren't arbitrary, they correspond to the 1, 4 and 8-bit
bus modes.

You may have added this squash a warning, but I suspect the pinctrl
configuration in your devicetree is incorrect.

Andrew

>          - EMMCG8
>          - ESPI
>          - ESPIALT
Willie Thai March 31, 2025, 5:18 p.m. UTC | #3
>> Add EMMCG5 enum to compatible list of pinctrl binding for emmc
>> enabling.
>> 
>> Cc: Andrew Jeffery <andrew@codeconstruct.com.au>
>> Signed-off-by: Willie Thai <wthai@nvidia.com>
>> ---
>>  .../devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml      | 1
>> +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git
>> a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
>> pinctrl.yaml
>> b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
>> pinctrl.yaml
>> index 80974c46f3ef..cb75e979f5e0 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
>> pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
>> pinctrl.yaml
>> @@ -276,6 +276,7 @@ additionalProperties:
>>          - BMCINT
>>          - EMMCG1
>>          - EMMCG4
>> +        - EMMCG5
> 
> What pin configuration does this correspond to for the eMMC controller?
> These groups aren't arbitrary, they correspond to the 1, 4 and 8-bit
> bus modes.
> 
> You may have added this squash a warning, but I suspect the pinctrl
> configuration in your devicetree is incorrect.
> 
> Andrew
> 

Thanks for your feedback !
We want to exclude AC5 pin in the default EMMCG4 pin group, because that pin is used for other purpose.
We define a new group called EMMCG5 as:
GROUP_DECL(EMMCG5, AB4, AA4, AC4, AA5, Y5, AB5, AB6)
The bus mode is still 4-bit mode.
Could you please advise if we can use the name "EMMCG5" ?

>>          - EMMCG8
>>          - ESPI
Andrew Jeffery April 1, 2025, 12:52 a.m. UTC | #4
Hi Willie,

On Mon, 2025-03-31 at 17:18 +0000, Willie Thai wrote:
> > > Add EMMCG5 enum to compatible list of pinctrl binding for emmc
> > > enabling.
> > > 
> > > Cc: Andrew Jeffery <andrew@codeconstruct.com.au>
> > > Signed-off-by: Willie Thai <wthai@nvidia.com>
> > > ---
> > >  .../devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml     
> > > | 1
> > > +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> > > pinctrl.yaml
> > > b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> > > pinctrl.yaml
> > > index 80974c46f3ef..cb75e979f5e0 100644
> > > --- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> > > pinctrl.yaml
> > > +++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-
> > > pinctrl.yaml
> > > @@ -276,6 +276,7 @@ additionalProperties:
> > >          - BMCINT
> > >          - EMMCG1
> > >          - EMMCG4
> > > +        - EMMCG5
> > 
> > What pin configuration does this correspond to for the eMMC
> > controller?
> > These groups aren't arbitrary, they correspond to the 1, 4 and 8-
> > bit
> > bus modes.
> > 
> > You may have added this squash a warning, but I suspect the pinctrl
> > configuration in your devicetree is incorrect.
> > 
> > Andrew
> > 
> 
> Thanks for your feedback !
> We want to exclude AC5 pin in the default EMMCG4 pin group, because
> that pin is used for other purpose.

Okay, sure.

> We define a new group called EMMCG5 as:
> GROUP_DECL(EMMCG5, AB4, AA4, AC4, AA5, Y5, AB5, AB6)
> The bus mode is still 4-bit mode.
> Could you please advise if we can use the name "EMMCG5" ?

Why is EMMCG5 an intuitive name? It doesn't make sense to me for what
you're trying to achieve. It's probably better if we rethink the
functions and groups to make them a little more fine-grained, perhaps

- EMMCDAT1
- EMMCDAT4
- EMMCDAT8
- EMMCWP
- EMMCCD

and then you request what's appropriate, rather than create groups that
exclude a specific function/pin (such as card detect).

Maybe you should drop the eMMC node from your devicetree for now, and
add it back once we've sorted out the pinctrl side of things in a
separate series.

Andrew
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
index 80974c46f3ef..cb75e979f5e0 100644
--- a/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/aspeed,ast2600-pinctrl.yaml
@@ -276,6 +276,7 @@  additionalProperties:
         - BMCINT
         - EMMCG1
         - EMMCG4
+        - EMMCG5
         - EMMCG8
         - ESPI
         - ESPIALT