Add DT bindings for BeaglePlay CC1352 co-processor.
The BeaglePlay has a CC1352 co-processor. This co-processor is 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
This commit adds net/ti,cc1352p7. It is used by gb-beagleplay greybus
driver.
Signed-off-by: Ayush Singh <[email protected]>
---
.../devicetree/bindings/net/ti,cc1352p7.yaml | 48 +++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 54 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..57bc2c43e5b1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
@@ -0,0 +1,48 @@
+# 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
+
+ reset-gpios:
+ maxItems: 1
+
+ power-gpios:
+ maxItems: 3
+ description:
+ The device has three power rails that are exposed on external pins VDDS,
+ VDDR and DCOUPL.
+
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ serial {
+ mcu {
+ compatible = "ti,cc1352p7";
+ clocks = <&sclk_hf 0>, <&sclk_lf 25>;
+ reset-gpios = <&pio 35 GPIO_ACTIVE_LOW>;
+ power-gpios = <&pio 1 GPIO_ACTIVE_HIGH>, <&pio 2 GPIO_ACTIVE_HIGH>, <&pio 3 GPIO_ACTIVE_HIGH>;
+ };
+ };
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
On 02/10/2023 20:24, Ayush Singh wrote:
> Add DT bindings for BeaglePlay CC1352 co-processor.
>
Thank you for your patch. There is something to discuss/improve.
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For example:
dt-bindings: net:
> The BeaglePlay has a CC1352 co-processor. This co-processor is 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
>
> This commit adds net/ti,cc1352p7. It is used by gb-beagleplay greybus
A nit: I pointed you to the documentation explaining not to use "This
commit adds". It's v6 and the wording is back. Instead drop both
sentences - they are pointless in this context. First one repeats
previous text, second describes driver, but we do not talk here about
drivers.
> driver.
>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> .../devicetree/bindings/net/ti,cc1352p7.yaml | 48 +++++++++++++++++++
> MAINTAINERS | 6 +++
> 2 files changed, 54 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..57bc2c43e5b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> @@ -0,0 +1,48 @@
> +# 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.
If over SPI, then the binding is incomplete. This is fine for now, I guess.
> +
> +maintainers:
> + - Ayush Singh <[email protected]>
> +
> +properties:
> + compatible:
> + const: ti,cc1352p7
> +
> + clocks:
> + maxItems: 2
Instead please list the items like:
clocks:
items:
- description: High-Foo-bar
- description: Low-Foo-bar
> +
> + reset-gpios:
> + maxItems: 1
> +
> + power-gpios:
> + maxItems: 3
> + description:
> + The device has three power rails that are exposed on external pins VDDS,
> + VDDR and DCOUPL.
No, power rails are not GPIOs. You need supplies, so:
vdds-supply: true
vddr-supply: true
dcoupl-supply: true
Look also at:
Documentation/devicetree/bindings/gpio/gpio-consumer-common.yaml
which explicitly allows only one powerdown GPIO.
> +
Best regards,
Krzysztof
On 23:54-20231002, Ayush Singh wrote:
> Add DT bindings for BeaglePlay CC1352 co-processor.
>
> The BeaglePlay has a CC1352 co-processor. This co-processor is 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
>
> This commit adds net/ti,cc1352p7. It is used by gb-beagleplay greybus
> driver.
>
> Signed-off-by: Ayush Singh <[email protected]>
> ---
> .../devicetree/bindings/net/ti,cc1352p7.yaml | 48 +++++++++++++++++++
> MAINTAINERS | 6 +++
> 2 files changed, 54 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..57bc2c43e5b1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> @@ -0,0 +1,48 @@
> +# 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
I would suggest clock-names and description for it.
> +
> + reset-gpios:
> + maxItems: 1
> +
> + power-gpios:
> + maxItems: 3
> + description:
> + The device has three power rails that are exposed on external pins VDDS,
> + VDDR and DCOUPL.
Shouldn't these be regulators? The power rails are input to the MCU,
correct?
The properties should be something like:
vdds-supply
vddr-supply
dcoupl-supply ? (not sure what dcoupl is, but description should provide
that info).
the gpio controls for those can be modelled by regulator-gpio ?
> +
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + serial {
> + mcu {
> + compatible = "ti,cc1352p7";
> + clocks = <&sclk_hf 0>, <&sclk_lf 25>;
> + reset-gpios = <&pio 35 GPIO_ACTIVE_LOW>;
> + power-gpios = <&pio 1 GPIO_ACTIVE_HIGH>, <&pio 2 GPIO_ACTIVE_HIGH>, <&pio 3 GPIO_ACTIVE_HIGH>;
> + };
> + };
> 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
>
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
>> driver.
>>
>> Signed-off-by: Ayush Singh <[email protected]>
>> ---
>> .../devicetree/bindings/net/ti,cc1352p7.yaml | 48 +++++++++++++++++++
>> MAINTAINERS | 6 +++
>> 2 files changed, 54 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..57bc2c43e5b1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
>> @@ -0,0 +1,48 @@
>> +# 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.
> If over SPI, then the binding is incomplete. This is fine for now, I guess.
>
> Best regards,
> Krzysztof
Well, I added the line about SPI because the data sheet states that
CC1352P7 can be connected over SPI or UART when used as wireless MCU.
But yes, I do not have much knowledge about SPI itself, so the bindings
might be incomplete for SPI usage. Should I remove it or leave it be?
Sincerely,
Ayush Singh
>> +
>> + reset-gpios:
>> + maxItems: 1
>> +
>> + power-gpios:
>> + maxItems: 3
>> + description:
>> + The device has three power rails that are exposed on external pins VDDS,
>> + VDDR and DCOUPL.
> Shouldn't these be regulators? The power rails are input to the MCU,
> correct?
> The properties should be something like:
> vdds-supply
> vddr-supply
> dcoupl-supply ? (not sure what dcoupl is, but description should provide
> that info).
>
> the gpio controls for those can be modelled by regulator-gpio ?
I picked up power lines from "CC13xx/CC26xx Hardware Configuration and
PCB Design Considerations Application Report" present under "8.14
Network Processor" of CC1352P7 data sheet.
But now looking closer, it doesn't seem like DCOUPL can be supplied
externally for CC1352P7 and thus should probably be removed.
Also, it seems like for CC1352P7, VDDR must always be supplied
internally (The data sheet states: "Internal supply, must be powered
from the internal DC/DC converter or the internal LDO"). Thus, it should
be safe to remove VDDR as well.
That means only VDDS needs to be present for power line.
CC13xx/CC26xx Hardware Configuration and PCB Design Considerations
Application Report: https://www.ti.com/lit/pdf/swra640
CC1352P7 Data sheet: https://www.ti.com/lit/gpn/CC1352P7
Sincerely,
Ayush Singh
On 18:17-20231003, Ayush Singh wrote:
> > > +
> > > + reset-gpios:
> > > + maxItems: 1
> > > +
> > > + power-gpios:
> > > + maxItems: 3
> > > + description:
> > > + The device has three power rails that are exposed on external pins VDDS,
> > > + VDDR and DCOUPL.
> > Shouldn't these be regulators? The power rails are input to the MCU,
> > correct?
> > The properties should be something like:
> > vdds-supply
> > vddr-supply
> > dcoupl-supply ? (not sure what dcoupl is, but description should provide
> > that info).
> >
> > the gpio controls for those can be modelled by regulator-gpio ?
>
> I picked up power lines from "CC13xx/CC26xx Hardware Configuration and PCB
> Design Considerations Application Report" present under "8.14 Network
> Processor" of CC1352P7 data sheet.
>
> But now looking closer, it doesn't seem like DCOUPL can be supplied
> externally for CC1352P7 and thus should probably be removed.
>
> Also, it seems like for CC1352P7, VDDR must always be supplied internally
> (The data sheet states: "Internal supply, must be powered from the internal
> DC/DC converter or the internal LDO"). Thus, it should be safe to remove
> VDDR as well.
From Figure 3-1. CC1312R 7x7 RF Part Schematic Overview (app report you
point out below)
Typical usage is vdds-supply supplying:
VDDS (pin 44)
VDDS2 (pin 13)
VDDS3 (pin 22)
VDDS_DCDC (pin 34)
And DCDC_SW (pin 33) supplies vddr supplying:
VDDR(pin 45)
VDDR_RF (Pin 48)
>
>
> That means only VDDS needs to be present for power line.
I agree that that would be the typical supply model. So, just
vdds-supply
>
>
> CC13xx/CC26xx Hardware Configuration and PCB Design Considerations
> Application Report: https://www.ti.com/lit/pdf/swra640
>
> CC1352P7 Data sheet: https://www.ti.com/lit/gpn/CC1352P7
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
On 17:39-20231003, Ayush Singh wrote:
> > > driver.
> > >
> > > Signed-off-by: Ayush Singh <[email protected]>
> > > ---
> > > .../devicetree/bindings/net/ti,cc1352p7.yaml | 48 +++++++++++++++++++
> > > MAINTAINERS | 6 +++
> > > 2 files changed, 54 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..57bc2c43e5b1
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/ti,cc1352p7.yaml
> > > @@ -0,0 +1,48 @@
> > > +# 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.
> > If over SPI, then the binding is incomplete. This is fine for now, I guess.
> >
> > Best regards,
> > Krzysztof
>
> Well, I added the line about SPI because the data sheet states that CC1352P7
> can be connected over SPI or UART when used as wireless MCU. But yes, I do
> not have much knowledge about SPI itself, so the bindings might be
> incomplete for SPI usage. Should I remove it or leave it be?
I'd suggest to leave it for now, we can expand as there is a need.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D