diff mbox series

[v3,3/7] dt-bindings: mfd: add binding for Apple Mac System Management Controller

Message ID E1osRXT-002mw3-JR@rmk-PC.armlinux.org.uk (mailing list archive)
State New, archived
Headers show
Series Add Apple Mac System Management Controller GPIOs | expand

Commit Message

Russell King (Oracle) Nov. 8, 2022, 4:33 p.m. UTC
Add a DT binding for the Apple Mac System Management Controller.

Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml

Comments

Linus Walleij Nov. 8, 2022, 8:42 p.m. UTC | #1
On Tue, Nov 8, 2022 at 5:33 PM Russell King (Oracle)
<rmk+kernel@armlinux.org.uk> wrote:

> Add a DT binding for the Apple Mac System Management Controller.
>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Looks good to me!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Krzysztof Kozlowski Nov. 8, 2022, 8:55 p.m. UTC | #2
On 08/11/2022 17:33, Russell King (Oracle) wrote:
> Add a DT binding for the Apple Mac System Management Controller.

Drop the second, redundant "binding" from subject. It's already in prefix.

> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> new file mode 100644
> index 000000000000..014eba5a1bbc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple Mac System Management Controller
> +
> +maintainers:
> +  - Hector Martin <marcan@marcan.st>
> +
> +description:
> +  Apple Mac System Management Controller implements various functions
> +  such as GPIO, RTC, power, reboot.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - apple,t6000-smc
> +          - apple,t8103-smc
> +          - apple,t8112-smc
> +      - const: apple,smc
> +
> +  reg:
> +    items:
> +      - description: SMC area
> +      - description: SRAM area
> +
> +  reg-names:
> +    items:
> +      - const: smc
> +      - const: sram
> +
> +  mboxes:
> +    maxItems: 1
> +
> +  gpio:
> +    $ref: /schemas/gpio/gpio-macsmc.yaml

So this depends on other patch, so:
1. You need mention the dependency in cover letter (nothing there),
2. Re-order patches.

The GPIO cannot go separate tree and this must be explicitly communicated.

> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - mboxes
> +
> +examples:
> +  - |
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      smc@23e400000 {

Usually these are called system-controller, to have a generic name (as
asked by DT spec).

> +        compatible = "apple,t8103-smc", "apple,smc";
> +        reg = <0x2 0x3e400000 0x0 0x4000>,
> +               <0x2 0x3fe00000 0x0 0x100000>;

Align the items (opening <).

With three above:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Russell King (Oracle) Nov. 8, 2022, 10:22 p.m. UTC | #3
On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
> On 08/11/2022 17:33, Russell King (Oracle) wrote:
> > Add a DT binding for the Apple Mac System Management Controller.
> 
> Drop the second, redundant "binding" from subject. It's already in prefix.

Yet another thing that's been there from the start... how many more
things are you going to pick up in subsequent versions of the patch?
When does this stop?

In any case, taking your comment literally,

"dt-bindings: mfd: add for Apple Mac System Management Controller"

makes no sense, so presumably you want something more than that.

In any case, I see several recent cases already merged which follow
the pattern that I've used and that you've reviewed.

> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > ---
> >  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > new file mode 100644
> > index 000000000000..014eba5a1bbc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > @@ -0,0 +1,67 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Apple Mac System Management Controller
> > +
> > +maintainers:
> > +  - Hector Martin <marcan@marcan.st>
> > +
> > +description:
> > +  Apple Mac System Management Controller implements various functions
> > +  such as GPIO, RTC, power, reboot.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - apple,t6000-smc
> > +          - apple,t8103-smc
> > +          - apple,t8112-smc
> > +      - const: apple,smc
> > +
> > +  reg:
> > +    items:
> > +      - description: SMC area
> > +      - description: SRAM area
> > +
> > +  reg-names:
> > +    items:
> > +      - const: smc
> > +      - const: sram
> > +
> > +  mboxes:
> > +    maxItems: 1
> > +
> > +  gpio:
> > +    $ref: /schemas/gpio/gpio-macsmc.yaml
> 
> So this depends on other patch, so:
> 1. You need mention the dependency in cover letter (nothing there),
> 2. Re-order patches.
> 
> The GPIO cannot go separate tree and this must be explicitly communicated.

Sigh, getting an order that is sensible is really bloody difficult.
I'm quite sure Lee is only going to want to apply the mfd bits. Then
what do we do with the other bits? GPIO stuff via the GPIO tree, then
wait a cycle before the rest can be merged. Or what?

> > +
> > +additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +  - mboxes
> > +
> > +examples:
> > +  - |
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      smc@23e400000 {
> 
> Usually these are called system-controller, to have a generic name (as
> asked by DT spec).

I'll defer to Hector for his response on this one, but you've had
had plenty of opportunities to bring this up in the past - it's been
there since the first posting.

Frustrating is definitely the word for this drip-drip-drip review.
Rob Herring Nov. 8, 2022, 10:30 p.m. UTC | #4
On Tue, 08 Nov 2022 16:33:27 +0000, Russell King (Oracle) wrote:
> Add a DT binding for the Apple Mac System Management Controller.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> ---
>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/mfd/apple,smc.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/gpio/gpio-macsmc.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/apple,smc.example.dtb: smc@23e400000: gpio: False schema does not allow {'gpio-controller': True, '#gpio-cells': [[2]]}
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/apple,smc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.
Krzysztof Kozlowski Nov. 9, 2022, 8:35 a.m. UTC | #5
On 08/11/2022 23:22, Russell King (Oracle) wrote:
> On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
>> On 08/11/2022 17:33, Russell King (Oracle) wrote:
>>> Add a DT binding for the Apple Mac System Management Controller.
>>
>> Drop the second, redundant "binding" from subject. It's already in prefix.
> 
> Yet another thing that's been there from the start... how many more
> things are you going to pick up in subsequent versions of the patch?
> When does this stop?
> 
> In any case, taking your comment literally,
> 
> "dt-bindings: mfd: add for Apple Mac System Management Controller"
> 
> makes no sense, so presumably you want something more than that.

dt-bindings: mfd: add Apple Mac System Management Controller

> 
> In any case, I see several recent cases already merged which follow
> the pattern that I've used and that you've reviewed.

Any many received comments to fix. I wouldn't mention it if there was no
second issue - your patchset is non-bisectable. Since you must resend,
please adjust the subject.

> 
>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>>> ---
>>>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
>>>  1 file changed, 67 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>> new file mode 100644
>>> index 000000000000..014eba5a1bbc
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>> @@ -0,0 +1,67 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Apple Mac System Management Controller
>>> +
>>> +maintainers:
>>> +  - Hector Martin <marcan@marcan.st>
>>> +
>>> +description:
>>> +  Apple Mac System Management Controller implements various functions
>>> +  such as GPIO, RTC, power, reboot.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - apple,t6000-smc
>>> +          - apple,t8103-smc
>>> +          - apple,t8112-smc
>>> +      - const: apple,smc
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: SMC area
>>> +      - description: SRAM area
>>> +
>>> +  reg-names:
>>> +    items:
>>> +      - const: smc
>>> +      - const: sram
>>> +
>>> +  mboxes:
>>> +    maxItems: 1
>>> +
>>> +  gpio:
>>> +    $ref: /schemas/gpio/gpio-macsmc.yaml
>>
>> So this depends on other patch, so:
>> 1. You need mention the dependency in cover letter (nothing there),
>> 2. Re-order patches.
>>
>> The GPIO cannot go separate tree and this must be explicitly communicated.
> 
> Sigh, getting an order that is sensible is really bloody difficult.
> I'm quite sure Lee is only going to want to apply the mfd bits. Then
> what do we do with the other bits? GPIO stuff via the GPIO tree, then
> wait a cycle before the rest can be merged. Or what?

It's nothing new... bindings headers, drivers and DTS all have it since
years. Your case is actually easy to solve:
1. Re-order patches, describe dependency, get ack from one maintainer so
it can go via other.
2. Squash patches and describe dependency. Only one maintainer will need
to pick it up (with second's ack).

I don't understand why we talk about such basics, you are far way more
experienced in kernel development than many of us...

Best regards,
Krzysztof
Rob Herring Nov. 9, 2022, 10:17 p.m. UTC | #6
On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
> On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
> > On 08/11/2022 17:33, Russell King (Oracle) wrote:
> > > Add a DT binding for the Apple Mac System Management Controller.
> > 
> > Drop the second, redundant "binding" from subject. It's already in prefix.
> 
> Yet another thing that's been there from the start... how many more
> things are you going to pick up in subsequent versions of the patch?
> When does this stop?
> 
> In any case, taking your comment literally,
> 
> "dt-bindings: mfd: add for Apple Mac System Management Controller"
> 
> makes no sense, so presumably you want something more than that.
> 
> In any case, I see several recent cases already merged which follow
> the pattern that I've used and that you've reviewed.
> 
> > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > ---
> > >  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
> > >  1 file changed, 67 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > new file mode 100644
> > > index 000000000000..014eba5a1bbc
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > @@ -0,0 +1,67 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Apple Mac System Management Controller
> > > +
> > > +maintainers:
> > > +  - Hector Martin <marcan@marcan.st>
> > > +
> > > +description:
> > > +  Apple Mac System Management Controller implements various functions
> > > +  such as GPIO, RTC, power, reboot.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - apple,t6000-smc
> > > +          - apple,t8103-smc
> > > +          - apple,t8112-smc
> > > +      - const: apple,smc
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: SMC area
> > > +      - description: SRAM area
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: smc
> > > +      - const: sram
> > > +
> > > +  mboxes:
> > > +    maxItems: 1
> > > +
> > > +  gpio:
> > > +    $ref: /schemas/gpio/gpio-macsmc.yaml
> > 
> > So this depends on other patch, so:
> > 1. You need mention the dependency in cover letter (nothing there),
> > 2. Re-order patches.
> > 
> > The GPIO cannot go separate tree and this must be explicitly communicated.
> 
> Sigh, getting an order that is sensible is really bloody difficult.

It's not. Sub-devices before the MFD. The only time that doesn't work is 
when the sub-devices put the parent MFD in their example. The solution 
there is don't do that. Just 1 complete example in the MFD schema and no 
examples in the sub-devices.

> I'm quite sure Lee is only going to want to apply the mfd bits. 

Indeed. I can't seem to make Lee care... All the schemas should really 
be applied together.

> Then
> what do we do with the other bits? GPIO stuff via the GPIO tree, then
> wait a cycle before the rest can be merged. Or what?

The schemas must be picked up in the same cycle. I don't care so much 
if subsystem maintainers' trees have warnings if they don't care, but I 
do care for linux-next. If the subsystem bits aren't picked up, then 
I'll pick them up if it comes to that.

Rob
Hector Martin Nov. 10, 2022, 11:35 a.m. UTC | #7
On 10/11/2022 07.17, Rob Herring wrote:
> On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
>> On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
>>> On 08/11/2022 17:33, Russell King (Oracle) wrote:
>>>> Add a DT binding for the Apple Mac System Management Controller.
>>>
>>> Drop the second, redundant "binding" from subject. It's already in prefix.
>>
>> Yet another thing that's been there from the start... how many more
>> things are you going to pick up in subsequent versions of the patch?
>> When does this stop?
>>
>> In any case, taking your comment literally,
>>
>> "dt-bindings: mfd: add for Apple Mac System Management Controller"
>>
>> makes no sense, so presumably you want something more than that.
>>
>> In any case, I see several recent cases already merged which follow
>> the pattern that I've used and that you've reviewed.
>>
>>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>>>> ---
>>>>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
>>>>  1 file changed, 67 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>> new file mode 100644
>>>> index 000000000000..014eba5a1bbc
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>> @@ -0,0 +1,67 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Apple Mac System Management Controller
>>>> +
>>>> +maintainers:
>>>> +  - Hector Martin <marcan@marcan.st>
>>>> +
>>>> +description:
>>>> +  Apple Mac System Management Controller implements various functions
>>>> +  such as GPIO, RTC, power, reboot.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    items:
>>>> +      - enum:
>>>> +          - apple,t6000-smc
>>>> +          - apple,t8103-smc
>>>> +          - apple,t8112-smc
>>>> +      - const: apple,smc
>>>> +
>>>> +  reg:
>>>> +    items:
>>>> +      - description: SMC area
>>>> +      - description: SRAM area
>>>> +
>>>> +  reg-names:
>>>> +    items:
>>>> +      - const: smc
>>>> +      - const: sram
>>>> +
>>>> +  mboxes:
>>>> +    maxItems: 1
>>>> +
>>>> +  gpio:
>>>> +    $ref: /schemas/gpio/gpio-macsmc.yaml
>>>
>>> So this depends on other patch, so:
>>> 1. You need mention the dependency in cover letter (nothing there),
>>> 2. Re-order patches.
>>>
>>> The GPIO cannot go separate tree and this must be explicitly communicated.
>>
>> Sigh, getting an order that is sensible is really bloody difficult.
> 
> It's not. Sub-devices before the MFD. The only time that doesn't work is 
> when the sub-devices put the parent MFD in their example. The solution 
> there is don't do that. Just 1 complete example in the MFD schema and no 
> examples in the sub-devices.
> 
>> I'm quite sure Lee is only going to want to apply the mfd bits. 
> 
> Indeed. I can't seem to make Lee care... All the schemas should really 
> be applied together.
> 
>> Then
>> what do we do with the other bits? GPIO stuff via the GPIO tree, then
>> wait a cycle before the rest can be merged. Or what?
> 
> The schemas must be picked up in the same cycle. I don't care so much 
> if subsystem maintainers' trees have warnings if they don't care, but I 
> do care for linux-next. If the subsystem bits aren't picked up, then 
> I'll pick them up if it comes to that.
> 

We can take all the schemas and DT changes via asahi-soc if that works
for you. This also lets us move forward with more related DT changes
that would apply on top of things already in that branch. Then Lee only
has to take the mfd core bits (and possibly the RTKit platform part, or
we can figure something else out for that).


- Hector
Russell King (Oracle) Nov. 10, 2022, 11:48 a.m. UTC | #8
On Wed, Nov 09, 2022 at 04:17:23PM -0600, Rob Herring wrote:
> On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
> > On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
> > > On 08/11/2022 17:33, Russell King (Oracle) wrote:
> > > > Add a DT binding for the Apple Mac System Management Controller.
> > > 
> > > Drop the second, redundant "binding" from subject. It's already in prefix.
> > 
> > Yet another thing that's been there from the start... how many more
> > things are you going to pick up in subsequent versions of the patch?
> > When does this stop?
> > 
> > In any case, taking your comment literally,
> > 
> > "dt-bindings: mfd: add for Apple Mac System Management Controller"
> > 
> > makes no sense, so presumably you want something more than that.
> > 
> > In any case, I see several recent cases already merged which follow
> > the pattern that I've used and that you've reviewed.
> > 
> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > ---
> > > >  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
> > > >  1 file changed, 67 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > > new file mode 100644
> > > > index 000000000000..014eba5a1bbc
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > > @@ -0,0 +1,67 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Apple Mac System Management Controller
> > > > +
> > > > +maintainers:
> > > > +  - Hector Martin <marcan@marcan.st>
> > > > +
> > > > +description:
> > > > +  Apple Mac System Management Controller implements various functions
> > > > +  such as GPIO, RTC, power, reboot.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +          - apple,t6000-smc
> > > > +          - apple,t8103-smc
> > > > +          - apple,t8112-smc
> > > > +      - const: apple,smc
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description: SMC area
> > > > +      - description: SRAM area
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: smc
> > > > +      - const: sram
> > > > +
> > > > +  mboxes:
> > > > +    maxItems: 1
> > > > +
> > > > +  gpio:
> > > > +    $ref: /schemas/gpio/gpio-macsmc.yaml
> > > 
> > > So this depends on other patch, so:
> > > 1. You need mention the dependency in cover letter (nothing there),
> > > 2. Re-order patches.
> > > 
> > > The GPIO cannot go separate tree and this must be explicitly communicated.
> > 
> > Sigh, getting an order that is sensible is really bloody difficult.
> 
> It's not. Sub-devices before the MFD. The only time that doesn't work is 
> when the sub-devices put the parent MFD in their example. The solution 
> there is don't do that. Just 1 complete example in the MFD schema and no 
> examples in the sub-devices.

Meanwhile, I was told by Krzysztof that DT schemas must always have an
example. So, different person, different story.

Anyway, I've washed my hands of this farce with this series. I'm not
planning to post another version of it. I've had enough of this crap.
Krzysztof Kozlowski Nov. 10, 2022, 2 p.m. UTC | #9
On 10/11/2022 12:48, Russell King (Oracle) wrote:
> On Wed, Nov 09, 2022 at 04:17:23PM -0600, Rob Herring wrote:
>> On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
>>> On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
>>>> On 08/11/2022 17:33, Russell King (Oracle) wrote:
>>>>> Add a DT binding for the Apple Mac System Management Controller.
>>>>
>>>> Drop the second, redundant "binding" from subject. It's already in prefix.
>>>
>>> Yet another thing that's been there from the start... how many more
>>> things are you going to pick up in subsequent versions of the patch?
>>> When does this stop?
>>>
>>> In any case, taking your comment literally,
>>>
>>> "dt-bindings: mfd: add for Apple Mac System Management Controller"
>>>
>>> makes no sense, so presumably you want something more than that.
>>>
>>> In any case, I see several recent cases already merged which follow
>>> the pattern that I've used and that you've reviewed.
>>>
>>>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>>>>> ---
>>>>>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
>>>>>  1 file changed, 67 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..014eba5a1bbc
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>>> @@ -0,0 +1,67 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Apple Mac System Management Controller
>>>>> +
>>>>> +maintainers:
>>>>> +  - Hector Martin <marcan@marcan.st>
>>>>> +
>>>>> +description:
>>>>> +  Apple Mac System Management Controller implements various functions
>>>>> +  such as GPIO, RTC, power, reboot.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    items:
>>>>> +      - enum:
>>>>> +          - apple,t6000-smc
>>>>> +          - apple,t8103-smc
>>>>> +          - apple,t8112-smc
>>>>> +      - const: apple,smc
>>>>> +
>>>>> +  reg:
>>>>> +    items:
>>>>> +      - description: SMC area
>>>>> +      - description: SRAM area
>>>>> +
>>>>> +  reg-names:
>>>>> +    items:
>>>>> +      - const: smc
>>>>> +      - const: sram
>>>>> +
>>>>> +  mboxes:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  gpio:
>>>>> +    $ref: /schemas/gpio/gpio-macsmc.yaml
>>>>
>>>> So this depends on other patch, so:
>>>> 1. You need mention the dependency in cover letter (nothing there),
>>>> 2. Re-order patches.
>>>>
>>>> The GPIO cannot go separate tree and this must be explicitly communicated.
>>>
>>> Sigh, getting an order that is sensible is really bloody difficult.
>>
>> It's not. Sub-devices before the MFD. The only time that doesn't work is 
>> when the sub-devices put the parent MFD in their example. The solution 
>> there is don't do that. Just 1 complete example in the MFD schema and no 
>> examples in the sub-devices.
> 
> Meanwhile, I was told by Krzysztof that DT schemas must always have an
> example. So, different person, different story.

Hm, where do you see a message I told you to always have examples? Maybe
in some discussion I mentioned that examples are desired, but not
always. There is no point in having example in MFD child device schema
if it is already part of the parent MFD binding, where it is actually
required for complete picture.

Best regards,
Krzysztof
Russell King (Oracle) Nov. 10, 2022, 2:14 p.m. UTC | #10
On Thu, Nov 10, 2022 at 03:00:16PM +0100, Krzysztof Kozlowski wrote:
> On 10/11/2022 12:48, Russell King (Oracle) wrote:
> > On Wed, Nov 09, 2022 at 04:17:23PM -0600, Rob Herring wrote:
> >> On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
> >>> On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
> >>>> On 08/11/2022 17:33, Russell King (Oracle) wrote:
> >>>>> Add a DT binding for the Apple Mac System Management Controller.
> >>>>
> >>>> Drop the second, redundant "binding" from subject. It's already in prefix.
> >>>
> >>> Yet another thing that's been there from the start... how many more
> >>> things are you going to pick up in subsequent versions of the patch?
> >>> When does this stop?
> >>>
> >>> In any case, taking your comment literally,
> >>>
> >>> "dt-bindings: mfd: add for Apple Mac System Management Controller"
> >>>
> >>> makes no sense, so presumably you want something more than that.
> >>>
> >>> In any case, I see several recent cases already merged which follow
> >>> the pattern that I've used and that you've reviewed.
> >>>
> >>>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> >>>>> ---
> >>>>>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
> >>>>>  1 file changed, 67 insertions(+)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..014eba5a1bbc
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> >>>>> @@ -0,0 +1,67 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: Apple Mac System Management Controller
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Hector Martin <marcan@marcan.st>
> >>>>> +
> >>>>> +description:
> >>>>> +  Apple Mac System Management Controller implements various functions
> >>>>> +  such as GPIO, RTC, power, reboot.
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    items:
> >>>>> +      - enum:
> >>>>> +          - apple,t6000-smc
> >>>>> +          - apple,t8103-smc
> >>>>> +          - apple,t8112-smc
> >>>>> +      - const: apple,smc
> >>>>> +
> >>>>> +  reg:
> >>>>> +    items:
> >>>>> +      - description: SMC area
> >>>>> +      - description: SRAM area
> >>>>> +
> >>>>> +  reg-names:
> >>>>> +    items:
> >>>>> +      - const: smc
> >>>>> +      - const: sram
> >>>>> +
> >>>>> +  mboxes:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  gpio:
> >>>>> +    $ref: /schemas/gpio/gpio-macsmc.yaml
> >>>>
> >>>> So this depends on other patch, so:
> >>>> 1. You need mention the dependency in cover letter (nothing there),
> >>>> 2. Re-order patches.
> >>>>
> >>>> The GPIO cannot go separate tree and this must be explicitly communicated.
> >>>
> >>> Sigh, getting an order that is sensible is really bloody difficult.
> >>
> >> It's not. Sub-devices before the MFD. The only time that doesn't work is 
> >> when the sub-devices put the parent MFD in their example. The solution 
> >> there is don't do that. Just 1 complete example in the MFD schema and no 
> >> examples in the sub-devices.
> > 
> > Meanwhile, I was told by Krzysztof that DT schemas must always have an
> > example. So, different person, different story.
> 
> Hm, where do you see a message I told you to always have examples? Maybe
> in some discussion I mentioned that examples are desired, but not
> always. There is no point in having example in MFD child device schema
> if it is already part of the parent MFD binding, where it is actually
> required for complete picture.

35ed6e48-40e6-eb14-72de-9a0a4f5b38f8@linaro.org

and

2e2356f2-ded1-3cbf-4456-20054a8defda@linaro.org

For the GPIO macsec binding. So I'm getting contradictory information.
First you say that I need an example in the gpio macsec DT binding
yaml document.

Now I'm told it should go in the parent.

Make up your bloody minds and stop pissing me about. This is why I've
given up trying to get this in.

Getting a consistent message would be nice, but it seems impossible.
Krzysztof Kozlowski Nov. 10, 2022, 2:21 p.m. UTC | #11
On 10/11/2022 15:14, Russell King (Oracle) wrote:
> On Thu, Nov 10, 2022 at 03:00:16PM +0100, Krzysztof Kozlowski wrote:
>> On 10/11/2022 12:48, Russell King (Oracle) wrote:
>>> On Wed, Nov 09, 2022 at 04:17:23PM -0600, Rob Herring wrote:
>>>> On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
>>>>> On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
>>>>>> On 08/11/2022 17:33, Russell King (Oracle) wrote:
>>>>>>> Add a DT binding for the Apple Mac System Management Controller.
>>>>>>
>>>>>> Drop the second, redundant "binding" from subject. It's already in prefix.
>>>>>
>>>>> Yet another thing that's been there from the start... how many more
>>>>> things are you going to pick up in subsequent versions of the patch?
>>>>> When does this stop?
>>>>>
>>>>> In any case, taking your comment literally,
>>>>>
>>>>> "dt-bindings: mfd: add for Apple Mac System Management Controller"
>>>>>
>>>>> makes no sense, so presumably you want something more than that.
>>>>>
>>>>> In any case, I see several recent cases already merged which follow
>>>>> the pattern that I've used and that you've reviewed.
>>>>>
>>>>>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
>>>>>>>  1 file changed, 67 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..014eba5a1bbc
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
>>>>>>> @@ -0,0 +1,67 @@
>>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>>> +%YAML 1.2
>>>>>>> +---
>>>>>>> +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>> +
>>>>>>> +title: Apple Mac System Management Controller
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> +  - Hector Martin <marcan@marcan.st>
>>>>>>> +
>>>>>>> +description:
>>>>>>> +  Apple Mac System Management Controller implements various functions
>>>>>>> +  such as GPIO, RTC, power, reboot.
>>>>>>> +
>>>>>>> +properties:
>>>>>>> +  compatible:
>>>>>>> +    items:
>>>>>>> +      - enum:
>>>>>>> +          - apple,t6000-smc
>>>>>>> +          - apple,t8103-smc
>>>>>>> +          - apple,t8112-smc
>>>>>>> +      - const: apple,smc
>>>>>>> +
>>>>>>> +  reg:
>>>>>>> +    items:
>>>>>>> +      - description: SMC area
>>>>>>> +      - description: SRAM area
>>>>>>> +
>>>>>>> +  reg-names:
>>>>>>> +    items:
>>>>>>> +      - const: smc
>>>>>>> +      - const: sram
>>>>>>> +
>>>>>>> +  mboxes:
>>>>>>> +    maxItems: 1
>>>>>>> +
>>>>>>> +  gpio:
>>>>>>> +    $ref: /schemas/gpio/gpio-macsmc.yaml
>>>>>>
>>>>>> So this depends on other patch, so:
>>>>>> 1. You need mention the dependency in cover letter (nothing there),
>>>>>> 2. Re-order patches.
>>>>>>
>>>>>> The GPIO cannot go separate tree and this must be explicitly communicated.
>>>>>
>>>>> Sigh, getting an order that is sensible is really bloody difficult.
>>>>
>>>> It's not. Sub-devices before the MFD. The only time that doesn't work is 
>>>> when the sub-devices put the parent MFD in their example. The solution 
>>>> there is don't do that. Just 1 complete example in the MFD schema and no 
>>>> examples in the sub-devices.
>>>
>>> Meanwhile, I was told by Krzysztof that DT schemas must always have an
>>> example. So, different person, different story.
>>
>> Hm, where do you see a message I told you to always have examples? Maybe
>> in some discussion I mentioned that examples are desired, but not
>> always. There is no point in having example in MFD child device schema
>> if it is already part of the parent MFD binding, where it is actually
>> required for complete picture.
> 
> 35ed6e48-40e6-eb14-72de-9a0a4f5b38f8@linaro.org
> 
> and

That was independent schema, no references to MFD, thus my comment. If
you post such stuff alone without indication it is part of MFD, what do
you expect from reviewers?

> 
> 2e2356f2-ded1-3cbf-4456-20054a8defda@linaro.org

Which was comment about MFD, right? It is expected to have example for
MFD. I never said it is mandatory for every schema, which you implied in
previous mailing.


> 
> For the GPIO macsec binding. So I'm getting contradictory information.
> First you say that I need an example in the gpio macsec DT binding
> yaml document.

First you split patches making reviewers life difficult. Then reviewers
don't get entire concept and they answer based what they got.

> 
> Now I'm told it should go in the parent.

After posting entire patchset with context you can get better review,
yes, that's right.

> 
> Make up your bloody minds and stop pissing me about. This is why I've
> given up trying to get this in.

I don't think it is constructive to discuss this with you anymore.

> 
> Getting a consistent message would be nice, but it seems impossible.
> 

Best regards,
Krzysztof
Russell King (Oracle) Nov. 10, 2022, 2:23 p.m. UTC | #12
On Thu, Nov 10, 2022 at 03:21:35PM +0100, Krzysztof Kozlowski wrote:
> On 10/11/2022 15:14, Russell King (Oracle) wrote:
> > On Thu, Nov 10, 2022 at 03:00:16PM +0100, Krzysztof Kozlowski wrote:
> >> On 10/11/2022 12:48, Russell King (Oracle) wrote:
> >>> On Wed, Nov 09, 2022 at 04:17:23PM -0600, Rob Herring wrote:
> >>>> On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
> >>>>> On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
> >>>>>> On 08/11/2022 17:33, Russell King (Oracle) wrote:
> >>>>>>> Add a DT binding for the Apple Mac System Management Controller.
> >>>>>>
> >>>>>> Drop the second, redundant "binding" from subject. It's already in prefix.
> >>>>>
> >>>>> Yet another thing that's been there from the start... how many more
> >>>>> things are you going to pick up in subsequent versions of the patch?
> >>>>> When does this stop?
> >>>>>
> >>>>> In any case, taking your comment literally,
> >>>>>
> >>>>> "dt-bindings: mfd: add for Apple Mac System Management Controller"
> >>>>>
> >>>>> makes no sense, so presumably you want something more than that.
> >>>>>
> >>>>> In any case, I see several recent cases already merged which follow
> >>>>> the pattern that I've used and that you've reviewed.
> >>>>>
> >>>>>>> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> >>>>>>> ---
> >>>>>>>  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
> >>>>>>>  1 file changed, 67 insertions(+)
> >>>>>>>  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> >>>>>>> new file mode 100644
> >>>>>>> index 000000000000..014eba5a1bbc
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> >>>>>>> @@ -0,0 +1,67 @@
> >>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>>>> +%YAML 1.2
> >>>>>>> +---
> >>>>>>> +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
> >>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>>>> +
> >>>>>>> +title: Apple Mac System Management Controller
> >>>>>>> +
> >>>>>>> +maintainers:
> >>>>>>> +  - Hector Martin <marcan@marcan.st>
> >>>>>>> +
> >>>>>>> +description:
> >>>>>>> +  Apple Mac System Management Controller implements various functions
> >>>>>>> +  such as GPIO, RTC, power, reboot.
> >>>>>>> +
> >>>>>>> +properties:
> >>>>>>> +  compatible:
> >>>>>>> +    items:
> >>>>>>> +      - enum:
> >>>>>>> +          - apple,t6000-smc
> >>>>>>> +          - apple,t8103-smc
> >>>>>>> +          - apple,t8112-smc
> >>>>>>> +      - const: apple,smc
> >>>>>>> +
> >>>>>>> +  reg:
> >>>>>>> +    items:
> >>>>>>> +      - description: SMC area
> >>>>>>> +      - description: SRAM area
> >>>>>>> +
> >>>>>>> +  reg-names:
> >>>>>>> +    items:
> >>>>>>> +      - const: smc
> >>>>>>> +      - const: sram
> >>>>>>> +
> >>>>>>> +  mboxes:
> >>>>>>> +    maxItems: 1
> >>>>>>> +
> >>>>>>> +  gpio:
> >>>>>>> +    $ref: /schemas/gpio/gpio-macsmc.yaml
> >>>>>>
> >>>>>> So this depends on other patch, so:
> >>>>>> 1. You need mention the dependency in cover letter (nothing there),
> >>>>>> 2. Re-order patches.
> >>>>>>
> >>>>>> The GPIO cannot go separate tree and this must be explicitly communicated.
> >>>>>
> >>>>> Sigh, getting an order that is sensible is really bloody difficult.
> >>>>
> >>>> It's not. Sub-devices before the MFD. The only time that doesn't work is 
> >>>> when the sub-devices put the parent MFD in their example. The solution 
> >>>> there is don't do that. Just 1 complete example in the MFD schema and no 
> >>>> examples in the sub-devices.
> >>>
> >>> Meanwhile, I was told by Krzysztof that DT schemas must always have an
> >>> example. So, different person, different story.
> >>
> >> Hm, where do you see a message I told you to always have examples? Maybe
> >> in some discussion I mentioned that examples are desired, but not
> >> always. There is no point in having example in MFD child device schema
> >> if it is already part of the parent MFD binding, where it is actually
> >> required for complete picture.
> > 
> > 35ed6e48-40e6-eb14-72de-9a0a4f5b38f8@linaro.org
> > 
> > and
> 
> That was independent schema, no references to MFD, thus my comment. If
> you post such stuff alone without indication it is part of MFD, what do
> you expect from reviewers?
> 
> > 
> > 2e2356f2-ded1-3cbf-4456-20054a8defda@linaro.org
> 
> Which was comment about MFD, right? It is expected to have example for
> MFD. I never said it is mandatory for every schema, which you implied in
> previous mailing.
> 
> 
> > 
> > For the GPIO macsec binding. So I'm getting contradictory information.
> > First you say that I need an example in the gpio macsec DT binding
> > yaml document.
> 
> First you split patches making reviewers life difficult. Then reviewers
> don't get entire concept and they answer based what they got.
> 
> > 
> > Now I'm told it should go in the parent.
> 
> After posting entire patchset with context you can get better review,
> yes, that's right.
> 
> > 
> > Make up your bloody minds and stop pissing me about. This is why I've
> > given up trying to get this in.
> 
> I don't think it is constructive to discuss this with you anymore.
> 
> > 
> > Getting a consistent message would be nice, but it seems impossible.
> > 
> 

In which case you CLEARLY didn't read the cover message to that two
patch series.

Again, YOU are giving contradictory information.
Krzysztof Kozlowski Nov. 10, 2022, 2:36 p.m. UTC | #13
On 10/11/2022 15:23, Russell King (Oracle) wrote:
>>>
>>> Make up your bloody minds and stop pissing me about. This is why I've
>>> given up trying to get this in.
>>
>> I don't think it is constructive to discuss this with you anymore.
>>
>>>
>>> Getting a consistent message would be nice, but it seems impossible.
>>>
>>
> 
> In which case you CLEARLY didn't read the cover message to that two
> patch series.

I have every day 100-200 new patches, so not every cover letter gets
enough attention. Your did not say this is part of MFD, just mentioned
some build dependency.

Anyway, job of submitter is to make the patch and its context readable
to the reviewers.

There is literally nothing about including in parent MFD node in DT.
Just take a look:

https://lore.kernel.org/all/Y1q5jW8ff0aUdjPd@shell.armlinux.org.uk/

Or maybe you refer to this:
"previously posted before the last merge window."
Yes, so I will go through hundreds of emails from a week or months to
satisfy you?

"this driver is dependent on the
Apple SMC driver in order to be buildable and usable."

The only hint... it's about driver, not MFD. Make it obvious for
readers, not obvious for you.

"It is expected
that this Apple SMC driver will be merged via Lee's MFD tree."

Maybe there is dependency, maybe not, who the heck knows.

> Again, YOU are giving contradictory information.

Best regards,
Krzysztof
Russell King (Oracle) Nov. 10, 2022, 2:43 p.m. UTC | #14
On Thu, Nov 10, 2022 at 03:36:17PM +0100, Krzysztof Kozlowski wrote:
> On 10/11/2022 15:23, Russell King (Oracle) wrote:
> >>>
> >>> Make up your bloody minds and stop pissing me about. This is why I've
> >>> given up trying to get this in.
> >>
> >> I don't think it is constructive to discuss this with you anymore.
> >>
> >>>
> >>> Getting a consistent message would be nice, but it seems impossible.
> >>>
> >>
> > 
> > In which case you CLEARLY didn't read the cover message to that two
> > patch series.
> 
> I have every day 100-200 new patches, so not every cover letter gets
> enough attention. Your did not say this is part of MFD, just mentioned
> some build dependency.
> 
> Anyway, job of submitter is to make the patch and its context readable
> to the reviewers.
> 
> There is literally nothing about including in parent MFD node in DT.
> Just take a look:
> 
> https://lore.kernel.org/all/Y1q5jW8ff0aUdjPd@shell.armlinux.org.uk/
> 
> Or maybe you refer to this:
> "previously posted before the last merge window."
> Yes, so I will go through hundreds of emails from a week or months to
> satisfy you?
> 
> "this driver is dependent on the
> Apple SMC driver in order to be buildable and usable."
> 
> The only hint... it's about driver, not MFD. Make it obvious for
> readers, not obvious for you.
> 
> "It is expected
> that this Apple SMC driver will be merged via Lee's MFD tree."
> 
> Maybe there is dependency, maybe not, who the heck knows.

Everything that needed to be said was said in the cover message.

I'm not going to bother continuing with this thread because there is
nothing more to be said. This is getting utterly idiotic.
Lee Jones Nov. 14, 2022, 10:05 a.m. UTC | #15
On Wed, 09 Nov 2022, Rob Herring wrote:

> On Tue, Nov 08, 2022 at 10:22:31PM +0000, Russell King (Oracle) wrote:
> > On Tue, Nov 08, 2022 at 09:55:58PM +0100, Krzysztof Kozlowski wrote:
> > > On 08/11/2022 17:33, Russell King (Oracle) wrote:
> > > > Add a DT binding for the Apple Mac System Management Controller.
> > > 
> > > Drop the second, redundant "binding" from subject. It's already in prefix.
> > 
> > Yet another thing that's been there from the start... how many more
> > things are you going to pick up in subsequent versions of the patch?
> > When does this stop?
> > 
> > In any case, taking your comment literally,
> > 
> > "dt-bindings: mfd: add for Apple Mac System Management Controller"
> > 
> > makes no sense, so presumably you want something more than that.
> > 
> > In any case, I see several recent cases already merged which follow
> > the pattern that I've used and that you've reviewed.
> > 
> > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> > > > ---
> > > >  .../devicetree/bindings/mfd/apple,smc.yaml    | 67 +++++++++++++++++++
> > > >  1 file changed, 67 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > > new file mode 100644
> > > > index 000000000000..014eba5a1bbc
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> > > > @@ -0,0 +1,67 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Apple Mac System Management Controller
> > > > +
> > > > +maintainers:
> > > > +  - Hector Martin <marcan@marcan.st>
> > > > +
> > > > +description:
> > > > +  Apple Mac System Management Controller implements various functions
> > > > +  such as GPIO, RTC, power, reboot.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    items:
> > > > +      - enum:
> > > > +          - apple,t6000-smc
> > > > +          - apple,t8103-smc
> > > > +          - apple,t8112-smc
> > > > +      - const: apple,smc
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description: SMC area
> > > > +      - description: SRAM area
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: smc
> > > > +      - const: sram
> > > > +
> > > > +  mboxes:
> > > > +    maxItems: 1
> > > > +
> > > > +  gpio:
> > > > +    $ref: /schemas/gpio/gpio-macsmc.yaml
> > > 
> > > So this depends on other patch, so:
> > > 1. You need mention the dependency in cover letter (nothing there),
> > > 2. Re-order patches.
> > > 
> > > The GPIO cannot go separate tree and this must be explicitly communicated.
> > 
> > Sigh, getting an order that is sensible is really bloody difficult.
> 
> It's not. Sub-devices before the MFD. The only time that doesn't work is 
> when the sub-devices put the parent MFD in their example. The solution 
> there is don't do that. Just 1 complete example in the MFD schema and no 
> examples in the sub-devices.
> 
> > I'm quite sure Lee is only going to want to apply the mfd bits. 
> 
> Indeed. I can't seem to make Lee care... All the schemas should really 
> be applied together.

I care about drivers.  Happy to take the set as a whole if it helps.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
new file mode 100644
index 000000000000..014eba5a1bbc
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
@@ -0,0 +1,67 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/apple,smc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple Mac System Management Controller
+
+maintainers:
+  - Hector Martin <marcan@marcan.st>
+
+description:
+  Apple Mac System Management Controller implements various functions
+  such as GPIO, RTC, power, reboot.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - apple,t6000-smc
+          - apple,t8103-smc
+          - apple,t8112-smc
+      - const: apple,smc
+
+  reg:
+    items:
+      - description: SMC area
+      - description: SRAM area
+
+  reg-names:
+    items:
+      - const: smc
+      - const: sram
+
+  mboxes:
+    maxItems: 1
+
+  gpio:
+    $ref: /schemas/gpio/gpio-macsmc.yaml
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - mboxes
+
+examples:
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      smc@23e400000 {
+        compatible = "apple,t8103-smc", "apple,smc";
+        reg = <0x2 0x3e400000 0x0 0x4000>,
+               <0x2 0x3fe00000 0x0 0x100000>;
+        reg-names = "smc", "sram";
+        mboxes = <&smc_mbox>;
+
+        smc_gpio: gpio {
+          gpio-controller;
+          #gpio-cells = <2>;
+        };
+      };
+    };