mbox series

[v5,0/9] Make atmel serial driver aware of GCLK

Message ID 20220922113347.144383-1-sergiu.moga@microchip.com
Headers show
Series Make atmel serial driver aware of GCLK | expand

Message

Sergiu Moga Sept. 22, 2022, 11:33 a.m. UTC
This series of patches introduces the GCLK as a clock source for
the baudrate generator of UART on sama5d2 SoCs. Unlike the serial mode of
the USART offered by FLEXCOM, the UART does not provide a fractional part
that can be added to the clock divisor to obtain a more accurate result,
which greatly decreases the flexibility available for producing a higher
variety of baudrates. Now, with the last patch of the series, the driver
will check for a GCLK in the DT. If provided, whenever `atmel_set_termios`
is called, unless there is a fractional part, the driver will compare the
error rate between the desired baudrate and the actual baudrate obtained
through each of the available clock sources and will choose the clock source
with the lowest error rate. While at it, convert the DT binding
for UART/USART to json-schema, update the FLEXCOM binding to reference the
new UART/USART binding (while differentiating between the SPI of USART and the
SPI of FLEXCOM), do some small DT related fixups and do some small driver
cleanup.

The DT bindings related patches of this patch series depend on this patch
series converting atmel-flexcom bindings to json-schema:
https://lore.kernel.org/linux-arm-kernel/20220916075744.1879428-1-kavyasree.kotagiri@microchip.com/

v1 -> v2:
- [PATCH 3] dt-bindings: mfd: atmel,sama5d2-flexcom: Add SPI child node ref
    binding:
	- use full schema paths

- [PATCH 5] dt-bindings: serial: atmel,at91-usart: convert to json-schema
	- only do what the commit says, split the addition of other compatibles
	(PATCH 6) and properties (PATCH 13) in other patches
	- remove unnecessary "|"'s
	- mention header in `atmel,usart-mode`'s description
	- place `if:` under `allOf:`
	- respect order of spi0's DT properties: compatible, then reg then the
	reset of properties

- two new baudrate clock source related patches:
  [PATCH 9] tty: serial: atmel: Add definition for GCLK as baudrate source clock
			+
  [PATCH 10] tty: serial: atmel: Define BRSRCCK bitmask of UART IP's Mode
    Register:
	- v1's bitfield definition of GCLK was wrong, so add two more patches:
		- one for the definition of GCLK of USART IP's
		- one for the definition of BRSRCCK bitmask and its bitfields
		for UART IP's

- a new cleanup related patch that introduces a new struct atmel_uart_port field:
  [PATCH 11] tty: serial: atmel: Only divide Clock Divisor if the IP is USART:
  	- this ensures a division by 8 which is unnecessary and unappliable to
	UART IP's is only done for USART IP's

- four new patches regarding DT fixes and a SPI binding update that I came
upon:
  [PATCH 1] spi: dt-bindings: atmel,at91rm9200-spi: Add DMA related properties
  [PATCH 2] ARM: dts: at91: sama7g5: Swap rx and tx for spi11
  [PATCH 4] ARM: dts: at91: sam9x60ek: Add DBGU compatibles to uart1
  [PATCH 6] dt-bindings: serial: atmel,at91-usart: Highlight SAM9X60 incremental

- [PATCH 12] tty: serial: atmel: Make the driver aware of the existence of GCLK
	- take into account the different placement of the baudrate clock source
	into the IP's Mode Register (USART vs UART)
	- don't check for atmel_port->gclk != NULL
	- use clk_round_rate instead of clk_set_rate + clk_get_rate
	- remove clk_disable_unprepare from the end of the probe method

v2 -> v3:
- Re-order the patches as suggested by Krzysztof Kozlowski:
1. DTS changes needed for aligning to schema.
2. all bindings
3. rest

- New DT consistency related patch:
  [PATCH 3] ARM: dts: at91: Add `atmel,usart-mode` required property to serial
    nodes

- [PATCH 6] dt-bindings: serial: atmel,at91-usart: convert to json-schema:
  - Check value of `atmel,usart-mode` instead of the node regex
  - Define all properties top level and disallow them explicitly for other type,
  since additionalProperties:false conflicts with referencing other schemas
  - Remove useless else if: after else:

- [PATCH 7] dt-bindings: serial: atmel,at91-usart: add SAM9260 compatibles to
  SAM9X60:
  - Use the commit message suggested by Krzysztof Kozlowski

- [PATCH 8] dt-bindings: mfd: atmel,sama5d2-flexcom: Add USART child node ref
  binding
  - Compare devices based on the compatible instead of the clock

- [PATCH 12] tty: serial: atmel: Only divide Clock Divisor if the IP is USART
  - Use ATMEL_US_CD instead of 65535

- [PATCH 14] tty: serial: atmel: Make the driver aware of the existence of GCLK
  - add `gclk_fail` goto
  - replace `goto err` with `goto err_clk_disable_unprepare;`


v3 -> v4:
- Remove the first 4 patches as they have already been applied
https://lore.kernel.org/linux-arm-kernel/b537bbcf-cb0f-551d-6dd0-cf50864bafa3@microchip.com/
https://lore.kernel.org/linux-arm-kernel/53e72e5d-47fc-403d-c969-61b267a9ff15@microchip.com/
https://lore.kernel.org/linux-arm-kernel/1ae89854-74fa-6194-304f-db31d56d3674@microchip.com/
https://lore.kernel.org/linux-arm-kernel/3234cd79-65db-1210-50c1-e880ec6d87a0@microchip.com/
- Remove the addition of gclk's to sama5d2 clock driver as it has already been applied
https://lore.kernel.org/linux-arm-kernel/4b23db7d-d6b2-6c93-01f7-6a3b86f403d1@microchip.com/
- [PATCH 2] -> [PATCH 5]
  - add Acked-by/Reviewed-by tags to DT bindings
- [PATCH 8]
  - replace & with min_t



v4 -> v5:
- squash previous
`[PATCH v4 7/9] tty: serial: atmel: Define BRSRCCK bitmask of UART IP's Mode Register`
into a newly added
`[PATCH v5 6/9] tty: serial: atmel: Separate mode clearing between UART and USART`
whose role is mainly of cleanup and to make a clear separation between the
clearing of the mode for UART vs USART and make BRSRCCK into a bitfield
instead of a bitmask as it is only a bit.
- squash previous
`[PATCH v4 6/9] tty: serial: atmel: Define GCLK as USART baudrate source clock`
into the current
`[PATCH v5 8/9] tty: serial: atmel: Make the driver aware of the existence of GCLK`
- new bitfield conversions to FIELD_PREP/FIELD_GET PATCH
`[PATCH v5 9/9] tty: serial: atmel: Use FIELD_PREP/FIELD_GET`


Sergiu Moga (9):
  dt-bindings: mfd: atmel,sama5d2-flexcom: Add SPI child node ref
    binding
  dt-bindings: serial: atmel,at91-usart: convert to json-schema
  dt-bindings: serial: atmel,at91-usart: Add SAM9260 compatibles to
    SAM9X60
  dt-bindings: mfd: atmel,sama5d2-flexcom: Add USART child node ref
    binding
  dt-bindings: serial: atmel,at91-usart: Add gclk as a possible USART
    clock
  tty: serial: atmel: Separate mode clearing between UART and USART
  tty: serial: atmel: Only divide Clock Divisor if the IP is USART
  tty: serial: atmel: Make the driver aware of the existence of GCLK
  tty: serial: atmel: Use FIELD_PREP/FIELD_GET

 .../bindings/mfd/atmel,sama5d2-flexcom.yaml   |  19 +-
 .../devicetree/bindings/mfd/atmel-usart.txt   |  98 ---------
 .../bindings/serial/atmel,at91-usart.yaml     | 190 ++++++++++++++++++
 drivers/tty/serial/atmel_serial.c             |  82 +++++++-
 drivers/tty/serial/atmel_serial.h             |  75 +++----
 5 files changed, 321 insertions(+), 143 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-usart.txt
 create mode 100644 Documentation/devicetree/bindings/serial/atmel,at91-usart.yaml

Comments

Lee Jones Sept. 28, 2022, 3:03 p.m. UTC | #1
On Thu, 22 Sep 2022, Sergiu Moga wrote:

> Another functionality of FLEXCOM is that of SPI. In order for
> the proper validation of the SPI children nodes through the binding
> to occur, the proper binding for SPI must be referenced.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
> 
> 
> v1 -> v2:
> - use full schema paths
> 
> 
> v2 -> v3:
> - Added Reviewed-by tag, previously this was [PATCH 3]
> 
> 
> v3 -> v4:
> - Nothing, previously this was [PATCH 5]
> 
> 
> v4 -> v5:
> - Nothing
> 
> 
> 
>  .../devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml       | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Not sure how these can be handled.

I guess I cannot take these until the other patches are applied.

NB: The patch doesn't apply cleanly anyway, so will need to be rebased.

> diff --git a/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml b/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml
> index 0c80f4e98c54..f283cfd84b2d 100644
> --- a/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml
> +++ b/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml
> @@ -78,10 +78,9 @@ patternProperties:
>        of USART bindings.
>  
>    "^spi@[0-9a-f]+$":
> -    type: object
> +    $ref: /schemas/spi/atmel,at91rm9200-spi.yaml
>      description:
> -      Child node describing SPI. See ../spi/spi_atmel.txt for details
> -      of SPI bindings.
> +      Child node describing SPI.
>  
>    "^i2c@[0-9a-f]+$":
>      $ref: /schemas/i2c/atmel,at91sam-i2c.yaml
Sergiu Moga Sept. 28, 2022, 3:07 p.m. UTC | #2
On 28.09.2022 18:03, Lee Jones wrote:
> On Thu, 22 Sep 2022, Sergiu Moga wrote:
> 
>> Another functionality of FLEXCOM is that of SPI. In order for
>> the proper validation of the SPI children nodes through the binding
>> to occur, the proper binding for SPI must be referenced.
>>
>> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>
>>
>> v1 -> v2:
>> - use full schema paths
>>
>>
>> v2 -> v3:
>> - Added Reviewed-by tag, previously this was [PATCH 3]
>>
>>
>> v3 -> v4:
>> - Nothing, previously this was [PATCH 5]
>>
>>
>> v4 -> v5:
>> - Nothing
>>
>>
>>
>>   .../devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml       | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> Not sure how these can be handled.
> 
> I guess I cannot take these until the other patches are applied.
> 
> NB: The patch doesn't apply cleanly anyway, so will need to be rebased.
> 


Hello,

The sama5d2-flexcom binding related patches are dependent on:
https://lore.kernel.org/linux-arm-kernel/20220916075744.1879428-1-kavyasree.kotagiri@microchip.com/

as specified in the cover letter.

Regards,
	Sergiu


>> diff --git a/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml b/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml
>> index 0c80f4e98c54..f283cfd84b2d 100644
>> --- a/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml
>> @@ -78,10 +78,9 @@ patternProperties:
>>         of USART bindings.
>>
>>     "^spi@[0-9a-f]+$":
>> -    type: object
>> +    $ref: /schemas/spi/atmel,at91rm9200-spi.yaml
>>       description:
>> -      Child node describing SPI. See ../spi/spi_atmel.txt for details
>> -      of SPI bindings.
>> +      Child node describing SPI.
>>
>>     "^i2c@[0-9a-f]+$":
>>       $ref: /schemas/i2c/atmel,at91sam-i2c.yaml
> 
> --
> Lee Jones [李琼斯]
Lee Jones Sept. 28, 2022, 3:23 p.m. UTC | #3
On Wed, 28 Sep 2022, Sergiu.Moga@microchip.com wrote:

> On 28.09.2022 18:03, Lee Jones wrote:
> > On Thu, 22 Sep 2022, Sergiu Moga wrote:
> > 
> >> Another functionality of FLEXCOM is that of SPI. In order for
> >> the proper validation of the SPI children nodes through the binding
> >> to occur, the proper binding for SPI must be referenced.
> >>
> >> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >> ---
> >>
> >>
> >> v1 -> v2:
> >> - use full schema paths
> >>
> >>
> >> v2 -> v3:
> >> - Added Reviewed-by tag, previously this was [PATCH 3]
> >>
> >>
> >> v3 -> v4:
> >> - Nothing, previously this was [PATCH 5]
> >>
> >>
> >> v4 -> v5:
> >> - Nothing
> >>
> >>
> >>
> >>   .../devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml       | 5 ++---
> >>   1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > Not sure how these can be handled.
> > 
> > I guess I cannot take these until the other patches are applied.
> > 
> > NB: The patch doesn't apply cleanly anyway, so will need to be rebased.
> > 
> 
> 
> Hello,
> 
> The sama5d2-flexcom binding related patches are dependent on:
> https://lore.kernel.org/linux-arm-kernel/20220916075744.1879428-1-kavyasree.kotagiri@microchip.com/

I would be very cautious about relying on comments made in the
cover-letter.  Better to make this a hard requirement and place them
in the same patch-set.
Sergiu Moga Sept. 28, 2022, 3:35 p.m. UTC | #4
On 28.09.2022 18:23, Lee Jones wrote:
> On Wed, 28 Sep 2022, Sergiu.Moga@microchip.com wrote:
> 
>> On 28.09.2022 18:03, Lee Jones wrote:
>>> On Thu, 22 Sep 2022, Sergiu Moga wrote:
>>>
>>>> Another functionality of FLEXCOM is that of SPI. In order for
>>>> the proper validation of the SPI children nodes through the binding
>>>> to occur, the proper binding for SPI must be referenced.
>>>>
>>>> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
>>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> ---
>>>>
>>>>
>>>> v1 -> v2:
>>>> - use full schema paths
>>>>
>>>>
>>>> v2 -> v3:
>>>> - Added Reviewed-by tag, previously this was [PATCH 3]
>>>>
>>>>
>>>> v3 -> v4:
>>>> - Nothing, previously this was [PATCH 5]
>>>>
>>>>
>>>> v4 -> v5:
>>>> - Nothing
>>>>
>>>>
>>>>
>>>>    .../devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml       | 5 ++---
>>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> Not sure how these can be handled.
>>>
>>> I guess I cannot take these until the other patches are applied.
>>>
>>> NB: The patch doesn't apply cleanly anyway, so will need to be rebased.
>>>
>>
>>
>> Hello,
>>
>> The sama5d2-flexcom binding related patches are dependent on:
>> https://lore.kernel.org/linux-arm-kernel/20220916075744.1879428-1-kavyasree.kotagiri@microchip.com/
> 
> I would be very cautious about relying on comments made in the
> cover-letter.  Better to make this a hard requirement and place them
> in the same patch-set.
> 
> --
> Lee Jones [李琼斯]



Understood, my apologies, I will keep this in mind the next time this 
happens :).

Otherwise, by applying the patch series linked above, my sama5d2-flexcom 
patches should apply cleanly afterwards.

Thanks,
	Sergiu
Claudiu Beznea Sept. 30, 2022, 7:22 a.m. UTC | #5
On 22.09.2022 14:33, Sergiu Moga wrote:
> Make sure that the driver only divides the clock divisor if the
> IP handled at that point is USART, since UART IP's do not support
> implicit peripheral clock division. Instead, in the case of UART,
> go with the highest possible clock divisor.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>


> ---
> 
> 
> v1 -> v2:
> - Nothing, this patch was not here before and is mainly meant as both cleanup
> and as a way to introduce a new field into struct atmel_uart_port that will be
> used by the last patch to diferentiate between USART and UART regarding the
> location of the Baudrate Clock Source bitmask.
> 
> 
> 
> v2 -> v3:
> - Use ATMEL_US_CD instead of 65535
> - Previously [PATCH 10]
> 
> 
> 
> v3 -> v4:
> - Use min_t instead of &
> - Previously [PATCH 12]
> 
> 
> v4 -> v5:
> - Added R-b tag
> 
> 
> 
>  drivers/tty/serial/atmel_serial.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index e3e14cb7668b..acbf6b82d687 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -150,6 +150,7 @@ struct atmel_uart_port {
>  	u32			rts_low;
>  	bool			ms_irq_enabled;
>  	u32			rtor;	/* address of receiver timeout register if it exists */
> +	bool			is_usart;
>  	bool			has_frac_baudrate;
>  	bool			has_hw_timer;
>  	struct timer_list	uart_timer;
> @@ -1825,6 +1826,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>  	 */
>  	atmel_port->has_frac_baudrate = false;
>  	atmel_port->has_hw_timer = false;
> +	atmel_port->is_usart = false;
>  
>  	if (name == new_uart) {
>  		dev_dbg(port->dev, "Uart with hw timer");
> @@ -1834,6 +1836,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>  		dev_dbg(port->dev, "Usart\n");
>  		atmel_port->has_frac_baudrate = true;
>  		atmel_port->has_hw_timer = true;
> +		atmel_port->is_usart = true;
>  		atmel_port->rtor = ATMEL_US_RTOR;
>  		version = atmel_uart_readl(port, ATMEL_US_VERSION);
>  		switch (version) {
> @@ -1863,6 +1866,7 @@ static void atmel_get_ip_name(struct uart_port *port)
>  			dev_dbg(port->dev, "This version is usart\n");
>  			atmel_port->has_frac_baudrate = true;
>  			atmel_port->has_hw_timer = true;
> +			atmel_port->is_usart = true;
>  			atmel_port->rtor = ATMEL_US_RTOR;
>  			break;
>  		case 0x203:
> @@ -2286,10 +2290,21 @@ static void atmel_set_termios(struct uart_port *port,
>  		cd = uart_get_divisor(port, baud);
>  	}
>  
> -	if (cd > 65535) {	/* BRGR is 16-bit, so switch to slower clock */
> +	/*
> +	 * If the current value of the Clock Divisor surpasses the 16 bit
> +	 * ATMEL_US_CD mask and the IP is USART, switch to the Peripheral
> +	 * Clock implicitly divided by 8.
> +	 * If the IP is UART however, keep the highest possible value for
> +	 * the CD and avoid needless division of CD, since UART IP's do not
> +	 * support implicit division of the Peripheral Clock.
> +	 */
> +	if (atmel_port->is_usart && cd > ATMEL_US_CD) {
>  		cd /= 8;
>  		mode |= ATMEL_US_USCLKS_MCK_DIV8;
> +	} else {
> +		cd = min_t(unsigned int, cd, ATMEL_US_CD);
>  	}
> +
>  	quot = cd | fp << ATMEL_US_FP_OFFSET;
>  
>  	if (!(port->iso7816.flags & SER_ISO7816_ENABLED))
Claudiu Beznea Sept. 30, 2022, 7:23 a.m. UTC | #6
On 22.09.2022 14:33, Sergiu Moga wrote:
> Convert all open-coded instances of bitfields retrieval/setting
> to FIELD_PREP/FIELD_GET where possible.
> 
> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>


> ---
> 
> 
> v1 -> v5:
> - Nothing, this patch was not here before
> 
> 
> 
>  drivers/tty/serial/atmel_serial.h | 74 ++++++++++++++++---------------
>  1 file changed, 38 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.h b/drivers/tty/serial/atmel_serial.h
> index 0fcadbeabc6c..87f8f7996307 100644
> --- a/drivers/tty/serial/atmel_serial.h
> +++ b/drivers/tty/serial/atmel_serial.h
> @@ -9,6 +9,8 @@
>   * Based on AT91RM9200 datasheet revision E.
>   */
>  
> +#include <linux/bitfield.h>
> +
>  #ifndef ATMEL_SERIAL_H
>  #define ATMEL_SERIAL_H
>  
> @@ -39,42 +41,42 @@
>  
>  #define ATMEL_US_MR		0x04	/* Mode Register */
>  #define	ATMEL_US_USMODE		GENMASK(3, 0)	/* Mode of the USART */
> -#define		ATMEL_US_USMODE_NORMAL		0
> -#define		ATMEL_US_USMODE_RS485		1
> -#define		ATMEL_US_USMODE_HWHS		2
> -#define		ATMEL_US_USMODE_MODEM		3
> -#define		ATMEL_US_USMODE_ISO7816_T0	4
> -#define		ATMEL_US_USMODE_ISO7816_T1	6
> -#define		ATMEL_US_USMODE_IRDA		8
> +#define		ATMEL_US_USMODE_NORMAL		FIELD_PREP(ATMEL_US_USMODE, 0)
> +#define		ATMEL_US_USMODE_RS485		FIELD_PREP(ATMEL_US_USMODE, 1)
> +#define		ATMEL_US_USMODE_HWHS		FIELD_PREP(ATMEL_US_USMODE, 2)
> +#define		ATMEL_US_USMODE_MODEM		FIELD_PREP(ATMEL_US_USMODE, 3)
> +#define		ATMEL_US_USMODE_ISO7816_T0	FIELD_PREP(ATMEL_US_USMODE, 4)
> +#define		ATMEL_US_USMODE_ISO7816_T1	FIELD_PREP(ATMEL_US_USMODE, 6)
> +#define		ATMEL_US_USMODE_IRDA		FIELD_PREP(ATMEL_US_USMODE, 8)
>  #define	ATMEL_US_USCLKS		GENMASK(5, 4)	/* Clock Selection */
> -#define		ATMEL_US_USCLKS_MCK		(0 <<  4)
> -#define		ATMEL_US_USCLKS_MCK_DIV8	(1 <<  4)
> -#define		ATMEL_US_USCLKS_GCLK		(2 <<  4)
> -#define		ATMEL_US_USCLKS_SCK		(3 <<  4)
> +#define		ATMEL_US_USCLKS_MCK		FIELD_PREP(ATMEL_US_USCLKS, 0)
> +#define		ATMEL_US_USCLKS_MCK_DIV8	FIELD_PREP(ATMEL_US_USCLKS, 1)
> +#define		ATMEL_US_USCLKS_GCLK		FIELD_PREP(ATMEL_US_USCLKS, 2)
> +#define		ATMEL_US_USCLKS_SCK		FIELD_PREP(ATMEL_US_USCLKS, 3)
>  #define	ATMEL_UA_FILTER		BIT(4)
>  #define	ATMEL_US_CHRL		GENMASK(7, 6)	/* Character Length */
> -#define		ATMEL_US_CHRL_5			(0 <<  6)
> -#define		ATMEL_US_CHRL_6			(1 <<  6)
> -#define		ATMEL_US_CHRL_7			(2 <<  6)
> -#define		ATMEL_US_CHRL_8			(3 <<  6)
> +#define		ATMEL_US_CHRL_5			FIELD_PREP(ATMEL_US_CHRL, 0)
> +#define		ATMEL_US_CHRL_6			FIELD_PREP(ATMEL_US_CHRL, 1)
> +#define		ATMEL_US_CHRL_7			FIELD_PREP(ATMEL_US_CHRL, 2)
> +#define		ATMEL_US_CHRL_8			FIELD_PREP(ATMEL_US_CHRL, 3)
>  #define	ATMEL_US_SYNC		BIT(8)		/* Synchronous Mode Select */
>  #define	ATMEL_US_PAR		GENMASK(11, 9)	/* Parity Type */
> -#define		ATMEL_US_PAR_EVEN		(0 <<  9)
> -#define		ATMEL_US_PAR_ODD		(1 <<  9)
> -#define		ATMEL_US_PAR_SPACE		(2 <<  9)
> -#define		ATMEL_US_PAR_MARK		(3 <<  9)
> -#define		ATMEL_US_PAR_NONE		(4 <<  9)
> -#define		ATMEL_US_PAR_MULTI_DROP		(6 <<  9)
> +#define		ATMEL_US_PAR_EVEN		FIELD_PREP(ATMEL_US_PAR, 0)
> +#define		ATMEL_US_PAR_ODD		FIELD_PREP(ATMEL_US_PAR, 1)
> +#define		ATMEL_US_PAR_SPACE		FIELD_PREP(ATMEL_US_PAR, 2)
> +#define		ATMEL_US_PAR_MARK		FIELD_PREP(ATMEL_US_PAR, 3)
> +#define		ATMEL_US_PAR_NONE		FIELD_PREP(ATMEL_US_PAR, 4)
> +#define		ATMEL_US_PAR_MULTI_DROP		FIELD_PREP(ATMEL_US_PAR, 6)
>  #define	ATMEL_US_NBSTOP		GENMASK(13, 12)	/* Number of Stop Bits */
> -#define		ATMEL_US_NBSTOP_1		(0 << 12)
> -#define		ATMEL_US_NBSTOP_1_5		(1 << 12)
> -#define		ATMEL_US_NBSTOP_2		(2 << 12)
> +#define		ATMEL_US_NBSTOP_1		FIELD_PREP(ATMEL_US_NBSTOP, 0)
> +#define		ATMEL_US_NBSTOP_1_5		FIELD_PREP(ATMEL_US_NBSTOP, 1)
> +#define		ATMEL_US_NBSTOP_2		FIELD_PREP(ATMEL_US_NBSTOP, 2)
>  #define	ATMEL_UA_BRSRCCK	BIT(12)	/* Clock Selection for UART */
>  #define	ATMEL_US_CHMODE		GENMASK(15, 14)	/* Channel Mode */
> -#define		ATMEL_US_CHMODE_NORMAL		(0 << 14)
> -#define		ATMEL_US_CHMODE_ECHO		(1 << 14)
> -#define		ATMEL_US_CHMODE_LOC_LOOP	(2 << 14)
> -#define		ATMEL_US_CHMODE_REM_LOOP	(3 << 14)
> +#define		ATMEL_US_CHMODE_NORMAL		FIELD_PREP(ATMEL_US_CHMODE, 0)
> +#define		ATMEL_US_CHMODE_ECHO		FIELD_PREP(ATMEL_US_CHMODE, 1)
> +#define		ATMEL_US_CHMODE_LOC_LOOP	FIELD_PREP(ATMEL_US_CHMODE, 2)
> +#define		ATMEL_US_CHMODE_REM_LOOP	FIELD_PREP(ATMEL_US_CHMODE, 3)
>  #define	ATMEL_US_MSBF		BIT(16)	/* Bit Order */
>  #define	ATMEL_US_MODE9		BIT(17)	/* 9-bit Character Length */
>  #define	ATMEL_US_CLKO		BIT(18)	/* Clock Output Select */
> @@ -82,7 +84,7 @@
>  #define	ATMEL_US_INACK		BIT(20)	/* Inhibit Non Acknowledge */
>  #define	ATMEL_US_DSNACK		BIT(21)	/* Disable Successive NACK */
>  #define	ATMEL_US_MAX_ITER_MASK	GENMASK(26, 24)	/* Max Iterations */
> -#define	ATMEL_US_MAX_ITER(n)	(((n) << 24) & ATMEL_US_MAX_ITER_MASK)
> +#define	ATMEL_US_MAX_ITER(n)	FIELD_PREP(ATMEL_US_MAX_ITER_MASK, (n))
>  #define	ATMEL_US_FILTER		BIT(28)	/* Infrared Receive Line Filter */
>  
>  #define ATMEL_US_IER		0x08	/* Interrupt Enable Register */
> @@ -134,19 +136,19 @@
>  
>  #define ATMEL_US_CMPR		0x90	/* Comparaison Register */
>  #define ATMEL_US_FMR		0xa0	/* FIFO Mode Register */
> -#define	ATMEL_US_TXRDYM(data)	(((data) & 0x3) << 0)	/* TX Ready Mode */
> -#define	ATMEL_US_RXRDYM(data)	(((data) & 0x3) << 4)	/* RX Ready Mode */
> +#define	ATMEL_US_TXRDYM(data)	FIELD_PREP(GENMASK(1, 0), (data))	/* TX Ready Mode */
> +#define	ATMEL_US_RXRDYM(data)	FIELD_PREP(GENMASK(5, 4), (data))	/* RX Ready Mode */
>  #define		ATMEL_US_ONE_DATA	0x0
>  #define		ATMEL_US_TWO_DATA	0x1
>  #define		ATMEL_US_FOUR_DATA	0x2
>  #define	ATMEL_US_FRTSC		BIT(7)	/* FIFO RTS pin Control */
> -#define	ATMEL_US_TXFTHRES(thr)	(((thr) & 0x3f) << 8)	/* TX FIFO Threshold */
> -#define	ATMEL_US_RXFTHRES(thr)	(((thr) & 0x3f) << 16)	/* RX FIFO Threshold */
> -#define	ATMEL_US_RXFTHRES2(thr)	(((thr) & 0x3f) << 24)	/* RX FIFO Threshold2 */
> +#define	ATMEL_US_TXFTHRES(thr)	FIELD_PREP(GENMASK(13, 8), (thr))	/* TX FIFO Threshold */
> +#define	ATMEL_US_RXFTHRES(thr)	FIELD_PREP(GENMASK(21, 16), (thr))	/* RX FIFO Threshold */
> +#define	ATMEL_US_RXFTHRES2(thr)	FIELD_PREP(GENMASK(29, 24), (thr))	/* RX FIFO Threshold2 */
>  
>  #define ATMEL_US_FLR		0xa4	/* FIFO Level Register */
> -#define	ATMEL_US_TXFL(reg)	(((reg) >> 0) & 0x3f)	/* TX FIFO Level */
> -#define	ATMEL_US_RXFL(reg)	(((reg) >> 16) & 0x3f)	/* RX FIFO Level */
> +#define	ATMEL_US_TXFL(reg)	FIELD_GET(GENMASK(5, 0), (reg))		/* TX FIFO Level */
> +#define	ATMEL_US_RXFL(reg)	FIELD_GET(GENMASK(21, 16), (reg))	/* RX FIFO Level */
>  
>  #define ATMEL_US_FIER		0xa8	/* FIFO Interrupt Enable Register */
>  #define ATMEL_US_FIDR		0xac	/* FIFO Interrupt Disable Register */
Lee Jones Oct. 24, 2022, 12:25 p.m. UTC | #7
On Wed, 28 Sep 2022, Sergiu.Moga@microchip.com wrote:

> On 28.09.2022 18:23, Lee Jones wrote:
> > On Wed, 28 Sep 2022, Sergiu.Moga@microchip.com wrote:
> > 
> >> On 28.09.2022 18:03, Lee Jones wrote:
> >>> On Thu, 22 Sep 2022, Sergiu Moga wrote:
> >>>
> >>>> Another functionality of FLEXCOM is that of SPI. In order for
> >>>> the proper validation of the SPI children nodes through the binding
> >>>> to occur, the proper binding for SPI must be referenced.
> >>>>
> >>>> Signed-off-by: Sergiu Moga <sergiu.moga@microchip.com>
> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >>>> ---
> >>>>
> >>>>
> >>>> v1 -> v2:
> >>>> - use full schema paths
> >>>>
> >>>>
> >>>> v2 -> v3:
> >>>> - Added Reviewed-by tag, previously this was [PATCH 3]
> >>>>
> >>>>
> >>>> v3 -> v4:
> >>>> - Nothing, previously this was [PATCH 5]
> >>>>
> >>>>
> >>>> v4 -> v5:
> >>>> - Nothing
> >>>>
> >>>>
> >>>>
> >>>>    .../devicetree/bindings/mfd/atmel,sama5d2-flexcom.yaml       | 5 ++---
> >>>>    1 file changed, 2 insertions(+), 3 deletions(-)
> >>>
> >>> Not sure how these can be handled.
> >>>
> >>> I guess I cannot take these until the other patches are applied.
> >>>
> >>> NB: The patch doesn't apply cleanly anyway, so will need to be rebased.
> >>>
> >>
> >>
> >> Hello,
> >>
> >> The sama5d2-flexcom binding related patches are dependent on:
> >> https://lore.kernel.org/linux-arm-kernel/20220916075744.1879428-1-kavyasree.kotagiri@microchip.com/
> > 
> > I would be very cautious about relying on comments made in the
> > cover-letter.  Better to make this a hard requirement and place them
> > in the same patch-set.
> > 
> 
> 
> 
> Understood, my apologies, I will keep this in mind the next time this 
> happens :).
> 
> Otherwise, by applying the patch series linked above, my sama5d2-flexcom 
> patches should apply cleanly afterwards.

Please re-send this once the other set has been applied.