mbox series

[v4,0/5] spi: dt-bindings: samsung: convert to dtschema

Message ID 20220119201005.13145-1-krzysztof.kozlowski@canonical.com
Headers show
Series spi: dt-bindings: samsung: convert to dtschema | expand

Message

Krzysztof Kozlowski Jan. 19, 2022, 8:10 p.m. UTC 
Hi,

Changes since v3
================
1. Patch 2: correct path in exynos-usi.yaml.
2. Add patch 5.
3. Add tags.

Changes since v2
================
1. Patch 2: drop child device schema, as Rob suggested.

Changes since v1
================
1. Patch 2: describe devices matching compatible, correct issues pointed out by
   Rob, add reviewed-by tag.
2. New patches 3 and 4.

Best regards,
Krzysztof

Krzysztof Kozlowski (5):
  ARM: dts: exynos: split dmas into array of phandles in Exynos5250
  spi: dt-bindings: samsung: convert to dtschema
  spi: dt-bindings: samsung: allow controller-data to be optional
  mfd: dt-bindings: google,cros-ec: reference Samsung SPI bindings
  spi: s3c64xx: allow controller-data to be optional

 .../bindings/mfd/google,cros-ec.yaml          |  29 +--
 .../bindings/soc/samsung/exynos-usi.yaml      |   2 +-
 .../spi/samsung,spi-peripheral-props.yaml     |  36 ++++
 .../devicetree/bindings/spi/samsung,spi.yaml  | 187 ++++++++++++++++++
 .../bindings/spi/spi-peripheral-props.yaml    |   1 +
 .../devicetree/bindings/spi/spi-samsung.txt   | 122 ------------
 MAINTAINERS                                   |   2 +-
 arch/arm/boot/dts/exynos5250.dtsi             |   9 +-
 drivers/spi/spi-s3c64xx.c                     |  14 +-
 9 files changed, 251 insertions(+), 151 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/samsung,spi-peripheral-props.yaml
 create mode 100644 Documentation/devicetree/bindings/spi/samsung,spi.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/spi-samsung.txt

Comments

Andi Shyti Jan. 19, 2022, 8:55 p.m. UTC | #1 
Hi Krzysztof,

On Wed, Jan 19, 2022 at 09:10:05PM +0100, Krzysztof Kozlowski wrote:
> The Samsung SoC SPI driver requires to provide controller-data node
> for each of SPI peripheral device nodes.  Make this controller-data node
> optional, so DTS could be simpler.
> 
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
> ---
>  drivers/spi/spi-s3c64xx.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
> index 8755cd85e83c..769d958a2f86 100644
> --- a/drivers/spi/spi-s3c64xx.c
> +++ b/drivers/spi/spi-s3c64xx.c
> @@ -796,16 +796,14 @@ static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	data_np = of_get_child_by_name(slave_np, "controller-data");
> -	if (!data_np) {
> -		dev_err(&spi->dev, "child node 'controller-data' not found\n");
> -		return ERR_PTR(-EINVAL);
> -	}
> -
>  	cs = kzalloc(sizeof(*cs), GFP_KERNEL);
> -	if (!cs) {
> -		of_node_put(data_np);
> +	if (!cs)
>  		return ERR_PTR(-ENOMEM);
> +
> +	data_np = of_get_child_by_name(slave_np, "controller-data");
> +	if (!data_np) {
> +		dev_info(&spi->dev, "child node 'controller-data' not found, using defaults\n");

"not found" sounds like an error; I would just write something
like "feedback delay set to '0' dfault", you also tell that the
default value is '0'.

In any case,

Reviewed-by: Andi Shyti <andi@etezian.org>

Andi
Krzysztof Kozlowski Jan. 20, 2022, 7:38 a.m. UTC | #2 
On 19/01/2022 21:55, Andi Shyti wrote:
> Hi Krzysztof,
> 
> On Wed, Jan 19, 2022 at 09:10:05PM +0100, Krzysztof Kozlowski wrote:
>> The Samsung SoC SPI driver requires to provide controller-data node
>> for each of SPI peripheral device nodes.  Make this controller-data node
>> optional, so DTS could be simpler.
>>
>> Suggested-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
>> Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
>> ---
>>  drivers/spi/spi-s3c64xx.c | 14 ++++++--------
>>  1 file changed, 6 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
>> index 8755cd85e83c..769d958a2f86 100644
>> --- a/drivers/spi/spi-s3c64xx.c
>> +++ b/drivers/spi/spi-s3c64xx.c
>> @@ -796,16 +796,14 @@ static struct s3c64xx_spi_csinfo *s3c64xx_get_slave_ctrldata(
>>  		return ERR_PTR(-EINVAL);
>>  	}
>>  
>> -	data_np = of_get_child_by_name(slave_np, "controller-data");
>> -	if (!data_np) {
>> -		dev_err(&spi->dev, "child node 'controller-data' not found\n");
>> -		return ERR_PTR(-EINVAL);
>> -	}
>> -
>>  	cs = kzalloc(sizeof(*cs), GFP_KERNEL);
>> -	if (!cs) {
>> -		of_node_put(data_np);
>> +	if (!cs)
>>  		return ERR_PTR(-ENOMEM);
>> +
>> +	data_np = of_get_child_by_name(slave_np, "controller-data");
>> +	if (!data_np) {
>> +		dev_info(&spi->dev, "child node 'controller-data' not found, using defaults\n");
> 
> "not found" sounds like an error; I would just write something
> like "feedback delay set to '0' dfault", you also tell that the
> default value is '0'.

Sure, I will rewrite the message.


Best regards,
Krzysztof