2023-10-04 18:47:08

by Ayush Singh

[permalink] [raw]
Subject: [PATCH v7 1/3] dt-bindings: net: Add ti,cc1352p7

Add DT bindings for Texas Instruments Simplelink CC1352P7 wireless MCU

BeaglePlay has CC1352P7 co-processor connected to the main AM62 (running
Linux) over UART. In the BeagleConnect Technology, CC1352 is responsible
for handling 6LoWPAN communication with beagleconnect freedom nodes as
well as their discovery.

It is used by gb-beagleplay greybus driver.

Signed-off-by: Ayush Singh <[email protected]>
---
.../devicetree/bindings/net/ti,cc1352p7.yaml | 51 +++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 57 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/ti,cc1352p7.yaml

diff --git a/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
new file mode 100644
index 000000000000..291ba34c389b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ti,cc1352p7.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments Simplelink CC1352P7 wireless MCU
+
+description:
+ The cc1352p7 mcu can be connected via SPI or UART.
+
+maintainers:
+ - Ayush Singh <[email protected]>
+
+properties:
+ compatible:
+ const: ti,cc1352p7
+
+ clocks:
+ maxItems: 2
+
+ clock-names:
+ description:
+ sclk_hf is the main system (mcu and peripherals) clock
+ sclk_lf is low-frequency system clock
+ items:
+ - const: sclk_hf
+ - const: sclk_lf
+
+ reset-gpios: true
+
+ vdds-supply: true
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ serial {
+ mcu {
+ compatible = "ti,cc1352p7";
+ clocks = <&sclk_hf 0>, <&sclk_lf 25>;
+ clock-names = "sclk_hf", "sclk_lf";
+ reset-gpios = <&pio 35 GPIO_ACTIVE_LOW>;
+ vdds-supply = <&vdds>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 37b9626ee654..5467669d7963 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8969,6 +8969,12 @@ F: drivers/staging/greybus/sdio.c
F: drivers/staging/greybus/spi.c
F: drivers/staging/greybus/spilib.c

+GREYBUS BEAGLEPLAY DRIVERS
+M: Ayush Singh <[email protected]>
+L: [email protected] (moderated for non-subscribers)
+S: Maintained
+F: Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
+
GREYBUS SUBSYSTEM
M: Johan Hovold <[email protected]>
M: Alex Elder <[email protected]>
--
2.41.0


2023-10-05 15:53:50

by Nishanth Menon

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] dt-bindings: net: Add ti,cc1352p7

On 13:51-20231005, Ayush Singh wrote:
> > > Signed-off-by: Ayush Singh <[email protected]>
> > > ---
> > > .../devicetree/bindings/net/ti,cc1352p7.yaml | 51 +++++++++++++++++++
> > > MAINTAINERS | 6 +++
> > > 2 files changed, 57 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> > > new file mode 100644
> > > index 000000000000..291ba34c389b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> > > @@ -0,0 +1,51 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/ti,cc1352p7.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Texas Instruments Simplelink CC1352P7 wireless MCU
> > > +
> > > +description:
> > > + The cc1352p7 mcu can be connected via SPI or UART.
> > > +
> > > +maintainers:
> > > + - Ayush Singh <[email protected]>
> > > +
> > > +properties:
> > > + compatible:
> > > + const: ti,cc1352p7
> > > +
> > > + clocks:
> > > + maxItems: 2
> > > +
> > > + clock-names:
> > > + description:
> > > + sclk_hf is the main system (mcu and peripherals) clock
> > > + sclk_lf is low-frequency system clock
> > This does no go here, but to clocks. I wrote how it should be done.
> > Don't ignore the feedback.
> It was suggested to use `clock-names` by Nishanth Menon in the previous
> email, so I thought this was what it meant. I will remove clock-names if
> that's better.

Krzysztof was mentioning that the description should be with clocks.
clock-names would allow for more descriptive dts

> > > + items:
> > > + - const: sclk_hf
> > > + - const: sclk_lf
> > > +
> > > + reset-gpios: true
> >
> > No, really, why do you change correct code into incorrect one? Who asked
> > you to drop maxItems?
> I found that many bindings (`display/ilitek,ili9486.yaml`,
> `iio/dac/adi,ad5758.yaml`) use this pattern instead of `maxItems` for
> `reset-gpios`. So I assumed it was some sort of convention. I will change it
> back to `maxItems`.

maxItems restrict the number of GPIOs to the ones that are actually
needed for the peripheral.

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2023-10-05 16:17:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] dt-bindings: net: Add ti,cc1352p7

On 05/10/2023 10:21, Ayush Singh wrote:

>>> + clocks:
>>> + maxItems: 2
>>> +
>>> + clock-names:
>>> + description:
>>> + sclk_hf is the main system (mcu and peripherals) clock
>>> + sclk_lf is low-frequency system clock
>> This does no go here, but to clocks. I wrote how it should be done.
>> Don't ignore the feedback.
> It was suggested to use `clock-names` by Nishanth Menon in the previous
> email, so I thought this was what it meant. I will remove clock-names if
> that's better.

clock-names could stay, but this description belongs to "clocks:" how I
wrote last time.

>>> + items:
>>> + - const: sclk_hf
>>> + - const: sclk_lf
>>> +
>>> + reset-gpios: true
>>
>> No, really, why do you change correct code into incorrect one? Who asked
>> you to drop maxItems?
> I found that many bindings (`display/ilitek,ili9486.yaml`,

Panels are constrained by panel-common.

> `iio/dac/adi,ad5758.yaml`) use this pattern instead of `maxItems` for

This I fixed now.


Best regards,
Krzysztof

2023-10-05 16:18:46

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] dt-bindings: net: Add ti,cc1352p7

On 04/10/2023 20:46, Ayush Singh wrote:
> Add DT bindings for Texas Instruments Simplelink CC1352P7 wireless MCU
>
> BeaglePlay has CC1352P7 co-processor connected to the main AM62 (running
> Linux) over UART. In the BeagleConnect Technology, CC1352 is responsible
> for handling 6LoWPAN communication with beagleconnect freedom nodes as
> well as their discovery.
>
> It is used by gb-beagleplay greybus driver.

"Instead drop both sentences"
>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> .../devicetree/bindings/net/ti,cc1352p7.yaml | 51 +++++++++++++++++++
> MAINTAINERS | 6 +++
> 2 files changed, 57 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> new file mode 100644
> index 000000000000..291ba34c389b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ti,cc1352p7.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments Simplelink CC1352P7 wireless MCU
> +
> +description:
> + The cc1352p7 mcu can be connected via SPI or UART.
> +
> +maintainers:
> + - Ayush Singh <[email protected]>
> +
> +properties:
> + compatible:
> + const: ti,cc1352p7
> +
> + clocks:
> + maxItems: 2
> +
> + clock-names:
> + description:
> + sclk_hf is the main system (mcu and peripherals) clock
> + sclk_lf is low-frequency system clock

This does no go here, but to clocks. I wrote how it should be done.
Don't ignore the feedback.

> + items:
> + - const: sclk_hf
> + - const: sclk_lf
> +
> + reset-gpios: true


No, really, why do you change correct code into incorrect one? Who asked
you to drop maxItems?

> +
> + vdds-supply: true

Best regards,
Krzysztof

2023-10-05 16:19:30

by Ayush Singh

[permalink] [raw]
Subject: Re: [PATCH v7 1/3] dt-bindings: net: Add ti,cc1352p7

>> Signed-off-by: Ayush Singh <[email protected]>
>> ---
>> .../devicetree/bindings/net/ti,cc1352p7.yaml | 51 +++++++++++++++++++
>> MAINTAINERS | 6 +++
>> 2 files changed, 57 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
>> new file mode 100644
>> index 000000000000..291ba34c389b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/ti,cc1352p7.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments Simplelink CC1352P7 wireless MCU
>> +
>> +description:
>> + The cc1352p7 mcu can be connected via SPI or UART.
>> +
>> +maintainers:
>> + - Ayush Singh <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + const: ti,cc1352p7
>> +
>> + clocks:
>> + maxItems: 2
>> +
>> + clock-names:
>> + description:
>> + sclk_hf is the main system (mcu and peripherals) clock
>> + sclk_lf is low-frequency system clock
> This does no go here, but to clocks. I wrote how it should be done.
> Don't ignore the feedback.
It was suggested to use `clock-names` by Nishanth Menon in the previous
email, so I thought this was what it meant. I will remove clock-names if
that's better.
>> + items:
>> + - const: sclk_hf
>> + - const: sclk_lf
>> +
>> + reset-gpios: true
>
> No, really, why do you change correct code into incorrect one? Who asked
> you to drop maxItems?
I found that many bindings (`display/ilitek,ili9486.yaml`,
`iio/dac/adi,ad5758.yaml`) use this pattern instead of `maxItems` for
`reset-gpios`. So I assumed it was some sort of convention. I will
change it back to `maxItems`.


Sincerely,

Ayush Singh