Add sam9x7 compatible to DT bindings documentation.
Signed-off-by: Varshini Rajendran <[email protected]>
---
Changes in v4:
- Fixed the wrong addition of compatible
- Added further compatibles that are possible correct (as per DT)
---
.../devicetree/bindings/serial/atmel,at91-usart.yaml | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
index 65cb2e5c5eee..30af537e8e81 100644
--- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
+++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
@@ -23,11 +23,17 @@ properties:
- const: atmel,at91sam9260-dbgu
- const: atmel,at91sam9260-usart
- items:
- - const: microchip,sam9x60-usart
+ - enum:
+ - microchip,sam9x60-usart
+ - microchip,sam9x7-usart
- const: atmel,at91sam9260-usart
- items:
- - const: microchip,sam9x60-dbgu
- - const: microchip,sam9x60-usart
+ - enum:
+ - microchip,sam9x60-dbgu
+ - microchip,sam9x7-dbgu
+ - enum:
+ - microchip,sam9x60-usart
+ - microchip,sam9x7-usart
- const: atmel,at91sam9260-dbgu
- const: atmel,at91sam9260-usart
--
2.25.1
On Fri, Feb 23, 2024 at 10:55:59PM +0530, Varshini Rajendran wrote:
> Add sam9x7 compatible to DT bindings documentation.
>
> Signed-off-by: Varshini Rajendran <[email protected]>
> ---
> Changes in v4:
> - Fixed the wrong addition of compatible
> - Added further compatibles that are possible correct (as per DT)
> ---
> .../devicetree/bindings/serial/atmel,at91-usart.yaml | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> index 65cb2e5c5eee..30af537e8e81 100644
> --- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> @@ -23,11 +23,17 @@ properties:
> - const: atmel,at91sam9260-dbgu
> - const: atmel,at91sam9260-usart
> - items:
> - - const: microchip,sam9x60-usart
> + - enum:
> + - microchip,sam9x60-usart
> + - microchip,sam9x7-usart
> - const: atmel,at91sam9260-usart
> - items:
> - - const: microchip,sam9x60-dbgu
> - - const: microchip,sam9x60-usart
> + - enum:
> + - microchip,sam9x60-dbgu
> + - microchip,sam9x7-dbgu
> + - enum:
> + - microchip,sam9x60-usart
> + - microchip,sam9x7-usart
This doesn't make sense - this enum should be a const.
I don't really understand the idea behind of the original binding here that
allowed:
"microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
Specifically, I don't get the purpose of the "microchip,sam9x60-usart".
Either make it
- items:
- enum:
- microchip,sam9x60-dbgu
- microchip,sam9x7-dbgu
- const: microchip,sam9x60-usart
- const: atmel,at91sam9260-dbgu
- const: atmel,at91sam9260-usart
or add
- items:
- const: microchip,sam9x60-dbgu
- const: atmel,at91sam9260-dbgu
- const: atmel,at91sam9260-usart
or explain exactly why this needs to be
"chipa-dgbu", "chipa-usart", "chipb-dbgu", "chipb-dbgu"
Thanks,
Conor.
Hi Conor,
On 25/02/24 1:32 am, Conor Dooley wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> On Fri, Feb 23, 2024 at 10:55:59PM +0530, Varshini Rajendran wrote:
>> Add sam9x7 compatible to DT bindings documentation.
>>
>> Signed-off-by: Varshini Rajendran <[email protected]>
>> ---
>> Changes in v4:
>> - Fixed the wrong addition of compatible
>> - Added further compatibles that are possible correct (as per DT)
>> ---
>> .../devicetree/bindings/serial/atmel,at91-usart.yaml | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>> index 65cb2e5c5eee..30af537e8e81 100644
>> --- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>> @@ -23,11 +23,17 @@ properties:
>> - const: atmel,at91sam9260-dbgu
>> - const: atmel,at91sam9260-usart
>> - items:
>> - - const: microchip,sam9x60-usart
>> + - enum:
>> + - microchip,sam9x60-usart
>> + - microchip,sam9x7-usart
>> - const: atmel,at91sam9260-usart
>> - items:
>> - - const: microchip,sam9x60-dbgu
>> - - const: microchip,sam9x60-usart
>> + - enum:
>> + - microchip,sam9x60-dbgu
>> + - microchip,sam9x7-dbgu
>
>> + - enum:
>> + - microchip,sam9x60-usart
>> + - microchip,sam9x7-usart
>
> This doesn't make sense - this enum should be a const.
> I don't really understand the idea behind of the original binding here that
> allowed:
> "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
>
> Specifically, I don't get the purpose of the "microchip,sam9x60-usart".
> Either make it
> - items:
> - enum:
> - microchip,sam9x60-dbgu
> - microchip,sam9x7-dbgu
> - const: microchip,sam9x60-usart
> - const: atmel,at91sam9260-dbgu
> - const: atmel,at91sam9260-usart
> or add
> - items:
> - const: microchip,sam9x60-dbgu
> - const: atmel,at91sam9260-dbgu
> - const: atmel,at91sam9260-usart
> or explain exactly why this needs to be
> "chipa-dgbu", "chipa-usart", "chipb-dbgu", "chipb-dbgu"
The compatible has to be "chipa-usart", "chipb-usart", "chipa-dbgu",
"chipb-dbgu" for the device to work as a debug console over UART
wher the chipa-<periph> is the device specific compatible
and the chipb-<periph> is the fallback compatible that the driver
actually uses.
Maybe putting the 2 compatibles as 2 enums is not right. I will rephrase
it as below.
- items:
- const: microchip,sam9x60-dbgu
- const: microchip,sam9x60-usart
- const: atmel,at91sam9260-dbgu
- const: atmel,at91sam9260-usart
- items:
- const: microchip,sam9x7-dbgu
- const: microchip,sam9x7-usart
- const: atmel,at91sam9260-dbgu
- const: atmel,at91sam9260-usart
Hope this is fine.
>
> Thanks,
> Conor.
>
--
Thanks and Regards,
Varshini Rajendran.
On Wed, Feb 28, 2024 at 07:03:01AM +0000, [email protected] wrote:
> Hi Conor,
>
> On 25/02/24 1:32 am, Conor Dooley wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > On Fri, Feb 23, 2024 at 10:55:59PM +0530, Varshini Rajendran wrote:
> >> Add sam9x7 compatible to DT bindings documentation.
> >>
> >> Signed-off-by: Varshini Rajendran <[email protected]>
> >> ---
> >> Changes in v4:
> >> - Fixed the wrong addition of compatible
> >> - Added further compatibles that are possible correct (as per DT)
> >> ---
> >> .../devicetree/bindings/serial/atmel,at91-usart.yaml | 12 +++++++++---
> >> 1 file changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> >> index 65cb2e5c5eee..30af537e8e81 100644
> >> --- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> >> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> >> @@ -23,11 +23,17 @@ properties:
> >> - const: atmel,at91sam9260-dbgu
> >> - const: atmel,at91sam9260-usart
> >> - items:
> >> - - const: microchip,sam9x60-usart
> >> + - enum:
> >> + - microchip,sam9x60-usart
> >> + - microchip,sam9x7-usart
> >> - const: atmel,at91sam9260-usart
> >> - items:
> >> - - const: microchip,sam9x60-dbgu
> >> - - const: microchip,sam9x60-usart
> >> + - enum:
> >> + - microchip,sam9x60-dbgu
> >> + - microchip,sam9x7-dbgu
> >
> >> + - enum:
> >> + - microchip,sam9x60-usart
> >> + - microchip,sam9x7-usart
> >
> > This doesn't make sense - this enum should be a const.
> > I don't really understand the idea behind of the original binding here that
> > allowed:
> > "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
> >
> > Specifically, I don't get the purpose of the "microchip,sam9x60-usart".
> > Either make it
> > - items:
> > - enum:
> > - microchip,sam9x60-dbgu
> > - microchip,sam9x7-dbgu
> > - const: microchip,sam9x60-usart
> > - const: atmel,at91sam9260-dbgu
> > - const: atmel,at91sam9260-usart
> > or add
> > - items:
> > - const: microchip,sam9x60-dbgu
> > - const: atmel,at91sam9260-dbgu
> > - const: atmel,at91sam9260-usart
> > or explain exactly why this needs to be
> > "chipa-dgbu", "chipa-usart", "chipb-dbgu", "chipb-dbgu"
> The compatible has to be "chipa-usart", "chipb-usart", "chipa-dbgu",
> "chipb-dbgu" for the device to work as a debug console over UART
> wher the chipa-<periph> is the device specific compatible
> and the chipb-<periph> is the fallback compatible that the driver
> actually uses.
This examples why you have "microchip,sam9x60-dbgu", "atmel,at91sam9260-dbgu"
and "atmel,at91sam9260-usart".
It does not explain "microchip,sam9x60-usart" though, I don't see what
purpose that serves. If used as a debug uart, you fall back to the
sam9260 debug uart compatible and if not you fall back to the sam9260
usart compatible.
In addition, the current setup implies that sam9x60 usart supports all
the features that the sam9260 debug usart does. I doubt that that is
true.
Thanks,
Conor.
Hi Conor,
On 28/02/24 5:19 pm, Conor Dooley wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> On Wed, Feb 28, 2024 at 07:03:01AM +0000, [email protected] wrote:
>> Hi Conor,
>>
>> On 25/02/24 1:32 am, Conor Dooley wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>> On Fri, Feb 23, 2024 at 10:55:59PM +0530, Varshini Rajendran wrote:
>>>> Add sam9x7 compatible to DT bindings documentation.
>>>>
>>>> Signed-off-by: Varshini Rajendran <[email protected]>
>>>> ---
>>>> Changes in v4:
>>>> - Fixed the wrong addition of compatible
>>>> - Added further compatibles that are possible correct (as per DT)
>>>> ---
>>>> .../devicetree/bindings/serial/atmel,at91-usart.yaml | 12 +++++++++---
>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>>>> index 65cb2e5c5eee..30af537e8e81 100644
>>>> --- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>>>> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
>>>> @@ -23,11 +23,17 @@ properties:
>>>> - const: atmel,at91sam9260-dbgu
>>>> - const: atmel,at91sam9260-usart
>>>> - items:
>>>> - - const: microchip,sam9x60-usart
>>>> + - enum:
>>>> + - microchip,sam9x60-usart
>>>> + - microchip,sam9x7-usart
>>>> - const: atmel,at91sam9260-usart
>>>> - items:
>>>> - - const: microchip,sam9x60-dbgu
>>>> - - const: microchip,sam9x60-usart
>>>> + - enum:
>>>> + - microchip,sam9x60-dbgu
>>>> + - microchip,sam9x7-dbgu
>>>
>>>> + - enum:
>>>> + - microchip,sam9x60-usart
>>>> + - microchip,sam9x7-usart
>>>
>>> This doesn't make sense - this enum should be a const.
>>> I don't really understand the idea behind of the original binding here that
>>> allowed:
>>> "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
>>>
>>> Specifically, I don't get the purpose of the "microchip,sam9x60-usart".
>>> Either make it
>>> - items:
>>> - enum:
>>> - microchip,sam9x60-dbgu
>>> - microchip,sam9x7-dbgu
>>> - const: microchip,sam9x60-usart
>>> - const: atmel,at91sam9260-dbgu
>>> - const: atmel,at91sam9260-usart
>>> or add
>>> - items:
>>> - const: microchip,sam9x60-dbgu
>>> - const: atmel,at91sam9260-dbgu
>>> - const: atmel,at91sam9260-usart
>>> or explain exactly why this needs to be
>>> "chipa-dgbu", "chipa-usart", "chipb-dbgu", "chipb-dbgu"
>> The compatible has to be "chipa-usart", "chipb-usart", "chipa-dbgu",
>> "chipb-dbgu" for the device to work as a debug console over UART
>> wher the chipa-<periph> is the device specific compatible
>> and the chipb-<periph> is the fallback compatible that the driver
>> actually uses.
>
> This examples why you have "microchip,sam9x60-dbgu", "atmel,at91sam9260-dbgu"
> and "atmel,at91sam9260-usart".
> It does not explain "microchip,sam9x60-usart" though, I don't see what
> purpose that serves. If used as a debug uart, you fall back to the
> sam9260 debug uart compatible and if not you fall back to the sam9260
> usart compatible.
>
Here, if it is not used as debug uart it has to fallback to the default
usart compatible which in this case should have a device specific
compatible too right?
The common usart compatible looks as follows,
compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
meaning the 1st one is the device specific usart compatible and the 2nd
one is the fallback compatible which the driver actually supports.
The debug uart looks as follows,
compatible = "microchip,sam9x60-dbgu", "atmel,at91sam9260-dbgu",
"microchip,sam9x60-usart", "atmel,at91sam9260-usart";
In this case, there is a device specific debug uart compatible, a
fallback tot he debug uart compatible and as you said if not used as a
debug uart it should fallback and work as a normal uart device which has
both a device specific compatible and a fallback to work.
In case the device specific compatible is supported with some other
features in the driver in the future, the debug uart also should get its
perk. Does this make sense?
> In addition, the current setup implies that sam9x60 usart supports all
> the features that the sam9260 debug usart does. I doubt that that is
> true.
>
> Thanks,
> Conor.
--
Thanks and Regards,
Varshini Rajendran.
On Thu, Feb 29, 2024 at 08:55:11AM +0000, [email protected] wrote:
> Hi Conor,
>
> On 28/02/24 5:19 pm, Conor Dooley wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > On Wed, Feb 28, 2024 at 07:03:01AM +0000, [email protected] wrote:
> >> Hi Conor,
> >>
> >> On 25/02/24 1:32 am, Conor Dooley wrote:
> >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >>> On Fri, Feb 23, 2024 at 10:55:59PM +0530, Varshini Rajendran wrote:
> >>>> Add sam9x7 compatible to DT bindings documentation.
> >>>>
> >>>> Signed-off-by: Varshini Rajendran <[email protected]>
> >>>> ---
> >>>> Changes in v4:
> >>>> - Fixed the wrong addition of compatible
> >>>> - Added further compatibles that are possible correct (as per DT)
> >>>> ---
> >>>> .../devicetree/bindings/serial/atmel,at91-usart.yaml | 12 +++++++++---
> >>>> 1 file changed, 9 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> >>>> index 65cb2e5c5eee..30af537e8e81 100644
> >>>> --- a/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> >>>> +++ b/Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml
> >>>> @@ -23,11 +23,17 @@ properties:
> >>>> - const: atmel,at91sam9260-dbgu
> >>>> - const: atmel,at91sam9260-usart
> >>>> - items:
> >>>> - - const: microchip,sam9x60-usart
> >>>> + - enum:
> >>>> + - microchip,sam9x60-usart
> >>>> + - microchip,sam9x7-usart
> >>>> - const: atmel,at91sam9260-usart
> >>>> - items:
> >>>> - - const: microchip,sam9x60-dbgu
> >>>> - - const: microchip,sam9x60-usart
> >>>> + - enum:
> >>>> + - microchip,sam9x60-dbgu
> >>>> + - microchip,sam9x7-dbgu
> >>>
> >>>> + - enum:
> >>>> + - microchip,sam9x60-usart
> >>>> + - microchip,sam9x7-usart
> >>>
> >>> This doesn't make sense - this enum should be a const.
> >>> I don't really understand the idea behind of the original binding here that
> >>> allowed:
> >>> "microchip,sam9x60-dbgu", "microchip,sam9x60-usart", "atmel,at91sam9260-dbgu", "atmel,at91sam9260-usart"
> >>>
> >>> Specifically, I don't get the purpose of the "microchip,sam9x60-usart".
> >>> Either make it
> >>> - items:
> >>> - enum:
> >>> - microchip,sam9x60-dbgu
> >>> - microchip,sam9x7-dbgu
> >>> - const: microchip,sam9x60-usart
> >>> - const: atmel,at91sam9260-dbgu
> >>> - const: atmel,at91sam9260-usart
> >>> or add
> >>> - items:
> >>> - const: microchip,sam9x60-dbgu
> >>> - const: atmel,at91sam9260-dbgu
> >>> - const: atmel,at91sam9260-usart
> >>> or explain exactly why this needs to be
> >>> "chipa-dgbu", "chipa-usart", "chipb-dbgu", "chipb-dbgu"
> >> The compatible has to be "chipa-usart", "chipb-usart", "chipa-dbgu",
> >> "chipb-dbgu" for the device to work as a debug console over UART
> >> wher the chipa-<periph> is the device specific compatible
> >> and the chipb-<periph> is the fallback compatible that the driver
> >> actually uses.
> >
> > This examples why you have "microchip,sam9x60-dbgu", "atmel,at91sam9260-dbgu"
> > and "atmel,at91sam9260-usart".
> > It does not explain "microchip,sam9x60-usart" though, I don't see what
> > purpose that serves. If used as a debug uart, you fall back to the
> > sam9260 debug uart compatible and if not you fall back to the sam9260
> > usart compatible.
> >
> Here, if it is not used as debug uart it has to fallback to the default
> usart compatible which in this case should have a device specific
> compatible too right?
>
> The common usart compatible looks as follows,
>
> compatible = "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
>
> meaning the 1st one is the device specific usart compatible and the 2nd
> one is the fallback compatible which the driver actually supports.
>
> The debug uart looks as follows,
>
> compatible = "microchip,sam9x60-dbgu", "atmel,at91sam9260-dbgu",
> "microchip,sam9x60-usart", "atmel,at91sam9260-usart";
This version here makes a lot more sense than what is currently in use
and what is being added in your original patch. I wouldn't object to
this being used.
> In this case, there is a device specific debug uart compatible, a
> fallback tot he debug uart compatible and as you said if not used as a
> debug uart it should fallback and work as a normal uart device which has
> both a device specific compatible and a fallback to work.
>
> In case the device specific compatible is supported with some other
> features in the driver in the future, the debug uart also should get its
> perk. Does this make sense?
>
>
> > In addition, the current setup implies that sam9x60 usart supports all
> > the features that the sam9260 debug usart does. I doubt that that is
> > true.