diff mbox series

[v4,1/3] dt-bindings: Add beaglecc1352

Message ID 20230902182845.1840620-2-ayushdevel1325@gmail.com
State Changes Requested
Headers show
Series greybus: Add BeaglePlay Greybus Driver | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Ayush Singh Sept. 2, 2023, 6:28 p.m. UTC 
Add DT bindings for BeagleCC1352 co-processor UART.

The BeaglePlay has a CC1352 co-processor. This co-processor is connected
to the main AM62 (running Linux) over UART. The CC1352 can run Zephyr
and other embedded OS. This commit adds DT bindings for the BeagleCC1352
UART, which will allow Linux platform drivers to identify and access this
device.

This commit adds serial/beaglecc1352 for identifying this UART. It is
used by an upcoming gb-beagleplay greybus driver.

Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
---
 .../bindings/serial/beaglecc1352.yaml         | 25 +++++++++++++++++++
 MAINTAINERS                                   |  6 +++++
 2 files changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/serial/beaglecc1352.yaml

Comments

Krzysztof Kozlowski Sept. 4, 2023, 7:14 a.m. UTC | #1 
On 02/09/2023 20:28, Ayush Singh wrote:
> Add DT bindings for BeagleCC1352 co-processor UART.

This does not look like UART controller.

> 
> The BeaglePlay has a CC1352 co-processor. This co-processor is connected
> to the main AM62 (running Linux) over UART. The CC1352 can run Zephyr
> and other embedded OS. This commit adds DT bindings for the BeagleCC1352

Please do not use "This commit/patch", but imperative mood. See longer
explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> UART, which will allow Linux platform drivers to identify and access this
> device.
> 
> This commit adds serial/beaglecc1352 for identifying this UART. It is
> used by an upcoming gb-beagleplay greybus driver.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

> 
> Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
> ---
>  .../bindings/serial/beaglecc1352.yaml         | 25 +++++++++++++++++++

It's not a serial driver. Don't put it in unrelated directory.

>  MAINTAINERS                                   |  6 +++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/serial/beaglecc1352.yaml
> 
> diff --git a/Documentation/devicetree/bindings/serial/beaglecc1352.yaml b/Documentation/devicetree/bindings/serial/beaglecc1352.yaml
> new file mode 100644
> index 000000000000..54db630a2a50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/beaglecc1352.yaml

Missing vendor prefix. Filename should match compatible. Compatible is
not "beaglecc1352"


> @@ -0,0 +1,25 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/serial/beaglecc1352.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: BeaglePlay CC1352 serial UART

How is this serial UART? Of what? The SoC? Do not describe interface but
the device.

> +
> +maintainers:
> +  - Ayush Singh <ayushdevel1325@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: beagle,cc1352

No resources? This does not seem useful... Put it then only in trivial
devices if your hardware - hardware, not driver - does not have any
pins, interrupts or other resources.

> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    beaglecc1352 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

Best regards,
Krzysztof
Ayush Singh Sept. 6, 2023, 4:47 p.m. UTC | #2 
On 9/4/23 12:44, Krzysztof Kozlowski wrote:
> On 02/09/2023 20:28, Ayush Singh wrote:
>> Add DT bindings for BeagleCC1352 co-processor UART.
> This does not look like UART controller.
>
>
>> Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
>> ---
>>   .../bindings/serial/beaglecc1352.yaml         | 25 +++++++++++++++++++
> It's not a serial driver. Don't put it in unrelated directory.
>
>> @@ -0,0 +1,25 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/serial/beaglecc1352.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: BeaglePlay CC1352 serial UART
> How is this serial UART? Of what? The SoC? Do not describe interface but
> the device.
>
>> +
>> +maintainers:
>> +  - Ayush Singh <ayushdevel1325@gmail.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: beagle,cc1352
> No resources? This does not seem useful... Put it then only in trivial
> devices if your hardware - hardware, not driver - does not have any
> pins, interrupts or other resources.
>
>> +
>> +required:
>> +  - compatible
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    beaglecc1352 {
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
> Best regards,
> Krzysztof

I would like to get some help on how to tackle this particular device 
since I cannot seem to find anything similar to this setup. First let me 
go over the setup.

The BeaglePlay board has 2 processors. AM625 processor which is the main 
processor. This runs the main Linux system. This processor does not have 
direct access to SubG.

It also contains a CC1352P7 MCU with it's own flash program memory, ROM 
and SRAM. This processor has SubG antenna. It runs it's own OS (Zephyr 
by default).

The only way for CC1352P7 and AM625 to communicate is by using that 
particular UART which is just a standard 8250 UART. The setup pretty 
much looks like 2 boards connected over UART except the UART will always 
be static.

I guess it would make most sense to write a CC1352P7 binding in this 
case? However, I am not sure how accurate that is since anything from 
the Linux side will be interacting with the Zephyr application, and not 
the processor. So in all actuality, the processor itself does not matter 
at all.


Ayush Singh
Krzysztof Kozlowski Sept. 11, 2023, 6:23 a.m. UTC | #3 
On 06/09/2023 18:47, Ayush Singh wrote:
> On 9/4/23 12:44, Krzysztof Kozlowski wrote:
>> On 02/09/2023 20:28, Ayush Singh wrote:
>>> Add DT bindings for BeagleCC1352 co-processor UART.
>> This does not look like UART controller.
>>
>>
>>> Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
>>> ---
>>>   .../bindings/serial/beaglecc1352.yaml         | 25 +++++++++++++++++++
>> It's not a serial driver. Don't put it in unrelated directory.
>>
>>> @@ -0,0 +1,25 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/serial/beaglecc1352.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: BeaglePlay CC1352 serial UART
>> How is this serial UART? Of what? The SoC? Do not describe interface but
>> the device.
>>
>>> +
>>> +maintainers:
>>> +  - Ayush Singh <ayushdevel1325@gmail.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: beagle,cc1352
>> No resources? This does not seem useful... Put it then only in trivial
>> devices if your hardware - hardware, not driver - does not have any
>> pins, interrupts or other resources.
>>
>>> +
>>> +required:
>>> +  - compatible
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    beaglecc1352 {
>> Node names should be generic. See also an explanation and list of
>> examples (not exhaustive) in DT specification:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>>
>> Best regards,
>> Krzysztof
> 
> I would like to get some help on how to tackle this particular device 
> since I cannot seem to find anything similar to this setup. First let me 
> go over the setup.
> 
> The BeaglePlay board has 2 processors. AM625 processor which is the main 
> processor. This runs the main Linux system. This processor does not have 
> direct access to SubG.
> 
> It also contains a CC1352P7 MCU with it's own flash program memory, ROM 
> and SRAM. This processor has SubG antenna. It runs it's own OS (Zephyr 
> by default).
> 
> The only way for CC1352P7 and AM625 to communicate is by using that 
> particular UART which is just a standard 8250 UART. The setup pretty 
> much looks like 2 boards connected over UART except the UART will always 
> be static.
> 
> I guess it would make most sense to write a CC1352P7 binding in this 
> case? However, I am not sure how accurate that is since anything from 
> the Linux side will be interacting with the Zephyr application, and not 
> the processor. So in all actuality, the processor itself does not matter 
> at all.

I think the bindings require specific name and give you proper hint what
should it be. If you still wonder, it means you still did not test your
DTS. Such testing is a requirement.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/serial/beaglecc1352.yaml b/Documentation/devicetree/bindings/serial/beaglecc1352.yaml
new file mode 100644
index 000000000000..54db630a2a50
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/beaglecc1352.yaml
@@ -0,0 +1,25 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/beaglecc1352.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: BeaglePlay CC1352 serial UART
+
+maintainers:
+  - Ayush Singh <ayushdevel1325@gmail.com>
+
+properties:
+  compatible:
+    const: beagle,cc1352
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    beaglecc1352 {
+      compatible = "beagle,cc1352";
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 37b9626ee654..9d1b49a6dfad 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 <ayushdevel1325@gmail.com>
+L:	greybus-dev@lists.linaro.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/serial/beaglecc1352.yaml
+
 GREYBUS SUBSYSTEM
 M:	Johan Hovold <johan@kernel.org>
 M:	Alex Elder <elder@kernel.org>