Add support for using GPIOs as chip select lines on Ingenic SoCs.
Signed-off-by: 周琰杰 (Zhou Yanjie) <[email protected]>
---
drivers/spi/spi-ingenic.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
index 03077a7..672e4ed 100644
--- a/drivers/spi/spi-ingenic.c
+++ b/drivers/spi/spi-ingenic.c
@@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct platform_device *pdev)
struct spi_controller *ctlr;
struct ingenic_spi *priv;
void __iomem *base;
- int ret;
+ int num_cs, ret;
pdata = of_device_get_match_data(dev);
if (!pdata) {
@@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct platform_device *pdev)
if (IS_ERR(priv->flen_field))
return PTR_ERR(priv->flen_field);
+ if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
+ dev_warn(dev, "Number of chip select lines not specified.\n");
+ num_cs = 2;
+ }
+
platform_set_drvdata(pdev, ctlr);
ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
@@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct platform_device *pdev)
ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
ctlr->min_speed_hz = 7200;
ctlr->max_speed_hz = 54000000;
- ctlr->num_chipselect = 2;
+ ctlr->use_gpio_descriptors = true;
+ ctlr->max_native_cs = 2;
+ ctlr->num_chipselect = num_cs;
ctlr->dev.of_node = pdev->dev.of_node;
if (spi_ingenic_request_dma(ctlr, dev))
--
2.7.4
Hi Zhou,
Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie)
<[email protected]> a écrit :
> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <[email protected]>
> ---
> drivers/spi/spi-ingenic.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
> index 03077a7..672e4ed 100644
> --- a/drivers/spi/spi-ingenic.c
> +++ b/drivers/spi/spi-ingenic.c
> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct
> platform_device *pdev)
> struct spi_controller *ctlr;
> struct ingenic_spi *priv;
> void __iomem *base;
> - int ret;
> + int num_cs, ret;
>
> pdata = of_device_get_match_data(dev);
> if (!pdata) {
> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct
> platform_device *pdev)
> if (IS_ERR(priv->flen_field))
> return PTR_ERR(priv->flen_field);
>
> + if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
> + dev_warn(dev, "Number of chip select lines not specified.\n");
> + num_cs = 2;
> + }
> +
> platform_set_drvdata(pdev, ctlr);
>
> ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct
> platform_device *pdev)
> ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
> ctlr->min_speed_hz = 7200;
> ctlr->max_speed_hz = 54000000;
> - ctlr->num_chipselect = 2;
> + ctlr->use_gpio_descriptors = true;
I wonder if this should be set conditionally instead. Maybe set it to
"true" if the "num-cs" property exists?
The rest looks good to me.
Cheers,
-Paul
> + ctlr->max_native_cs = 2;
> + ctlr->num_chipselect = num_cs;
> ctlr->dev.of_node = pdev->dev.of_node;
>
> if (spi_ingenic_request_dma(ctlr, dev))
> --
> 2.7.4
>
Hi Paul,
On 2022/4/15 下午11:00, Paul Cercueil wrote:
> Hi Zhou,
>
> Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie)
> <[email protected]> a écrit :
>> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <[email protected]>
>> ---
>> drivers/spi/spi-ingenic.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
>> index 03077a7..672e4ed 100644
>> --- a/drivers/spi/spi-ingenic.c
>> +++ b/drivers/spi/spi-ingenic.c
>> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct
>> platform_device *pdev)
>> struct spi_controller *ctlr;
>> struct ingenic_spi *priv;
>> void __iomem *base;
>> - int ret;
>> + int num_cs, ret;
>>
>> pdata = of_device_get_match_data(dev);
>> if (!pdata) {
>> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct
>> platform_device *pdev)
>> if (IS_ERR(priv->flen_field))
>> return PTR_ERR(priv->flen_field);
>>
>> + if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
>> + dev_warn(dev, "Number of chip select lines not specified.\n");
>> + num_cs = 2;
>> + }
>> +
>> platform_set_drvdata(pdev, ctlr);
>>
>> ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
>> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct
>> platform_device *pdev)
>> ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>> ctlr->min_speed_hz = 7200;
>> ctlr->max_speed_hz = 54000000;
>> - ctlr->num_chipselect = 2;
>> + ctlr->use_gpio_descriptors = true;
>
> I wonder if this should be set conditionally instead. Maybe set it to
> "true" if the "num-cs" property exists?
>
I'm not too sure, but it seems some other drivers like "spi-sun6i.c",
"spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it unconditionally.
> The rest looks good to me.
>
> Cheers,
> -Paul
>
>> + ctlr->max_native_cs = 2;
>> + ctlr->num_chipselect = num_cs;
>> ctlr->dev.of_node = pdev->dev.of_node;
>>
>> if (spi_ingenic_request_dma(ctlr, dev))
>> --
>> 2.7.4
>>
>
Hi Zhou,
Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie
<[email protected]> a écrit :
> Hi Paul,
>
> On 2022/4/15 下午11:00, Paul Cercueil wrote:
>> Hi Zhou,
>>
>> Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie)
>> <[email protected]> a écrit :
>>> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>>>
>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <[email protected]>
>>> ---
>>> drivers/spi/spi-ingenic.c | 11 +++++++++--
>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
>>> index 03077a7..672e4ed 100644
>>> --- a/drivers/spi/spi-ingenic.c
>>> +++ b/drivers/spi/spi-ingenic.c
>>> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct
>>> platform_device *pdev)
>>> struct spi_controller *ctlr;
>>> struct ingenic_spi *priv;
>>> void __iomem *base;
>>> - int ret;
>>> + int num_cs, ret;
>>>
>>> pdata = of_device_get_match_data(dev);
>>> if (!pdata) {
>>> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct
>>> platform_device *pdev)
>>> if (IS_ERR(priv->flen_field))
>>> return PTR_ERR(priv->flen_field);
>>>
>>> + if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
>>> + dev_warn(dev, "Number of chip select lines not
>>> specified.\n");
>>> + num_cs = 2;
>>> + }
>>> +
>>> platform_set_drvdata(pdev, ctlr);
>>>
>>> ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
>>> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct
>>> platform_device *pdev)
>>> ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>>> ctlr->min_speed_hz = 7200;
>>> ctlr->max_speed_hz = 54000000;
>>> - ctlr->num_chipselect = 2;
>>> + ctlr->use_gpio_descriptors = true;
>>
>> I wonder if this should be set conditionally instead. Maybe set it
>> to "true" if the "num-cs" property exists?
>>
>
> I'm not too sure, but it seems some other drivers like "spi-sun6i.c",
> "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it
> unconditionally.
Ok, maybe Mark can enlighten us here.
Cheers,
-Paul
>> The rest looks good to me.
>>
>> Cheers,
>> -Paul
>>
>>> + ctlr->max_native_cs = 2;
>>> + ctlr->num_chipselect = num_cs;
>>> ctlr->dev.of_node = pdev->dev.of_node;
>>>
>>> if (spi_ingenic_request_dma(ctlr, dev))
>>> --
>>> 2.7.4
>>>
>>
On Sat, Apr 16, 2022 at 05:36:46PM +0100, Paul Cercueil wrote:
> Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie
> > On 2022/4/15 下午11:00, Paul Cercueil wrote:
> > > > - ctlr->num_chipselect = 2;
> > > > + ctlr->use_gpio_descriptors = true;
> > > I wonder if this should be set conditionally instead. Maybe set it
> > > to "true" if the "num-cs" property exists?
> > I'm not too sure, but it seems some other drivers like "spi-sun6i.c",
> > "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it
> > unconditionally.
> Ok, maybe Mark can enlighten us here.
use_gpio_descriptions is just selecting which version of the GPIO APIs
we should use if we're handling GPIOs rather than if we should handle
them. We've got one last driver using the numerical GPIO APIs, once
that one is converted we should just be able to drop the flag since
everything will be using descriptors.
Hi Zhou,
Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie)
<[email protected]> a écrit :
> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <[email protected]>
> ---
> drivers/spi/spi-ingenic.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
> index 03077a7..672e4ed 100644
> --- a/drivers/spi/spi-ingenic.c
> +++ b/drivers/spi/spi-ingenic.c
> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct
> platform_device *pdev)
> struct spi_controller *ctlr;
> struct ingenic_spi *priv;
> void __iomem *base;
> - int ret;
> + int num_cs, ret;
>
> pdata = of_device_get_match_data(dev);
> if (!pdata) {
> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct
> platform_device *pdev)
> if (IS_ERR(priv->flen_field))
> return PTR_ERR(priv->flen_field);
>
> + if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
One small comment here - I think it would be better to use
device_property_read_u32().
The driver should also use device_get_match_data() instead of
of_device_get_match_data(), but that's a cleanup that can be done later.
Cheers,
-Paul
> + dev_warn(dev, "Number of chip select lines not specified.\n");
> + num_cs = 2;
> + }
> +
> platform_set_drvdata(pdev, ctlr);
>
> ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct
> platform_device *pdev)
> ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
> ctlr->min_speed_hz = 7200;
> ctlr->max_speed_hz = 54000000;
> - ctlr->num_chipselect = 2;
> + ctlr->use_gpio_descriptors = true;
> + ctlr->max_native_cs = 2;
> + ctlr->num_chipselect = num_cs;
> ctlr->dev.of_node = pdev->dev.of_node;
>
> if (spi_ingenic_request_dma(ctlr, dev))
> --
> 2.7.4
>
Hi,
Le mar., avril 19 2022 at 18:00:41 +0100, Mark Brown
<[email protected]> a écrit :
> On Sat, Apr 16, 2022 at 05:36:46PM +0100, Paul Cercueil wrote:
>> Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie
>> > On 2022/4/15 下午11:00, Paul Cercueil wrote:
>
>> > > > - ctlr->num_chipselect = 2;
>> > > > + ctlr->use_gpio_descriptors = true;
>
>> > > I wonder if this should be set conditionally instead. Maybe set
>> it
>> > > to "true" if the "num-cs" property exists?
>
>> > I'm not too sure, but it seems some other drivers like
>> "spi-sun6i.c",
>> > "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it
>> > unconditionally.
>
>> Ok, maybe Mark can enlighten us here.
>
> use_gpio_descriptions is just selecting which version of the GPIO APIs
> we should use if we're handling GPIOs rather than if we should handle
> them. We've got one last driver using the numerical GPIO APIs, once
> that one is converted we should just be able to drop the flag since
> everything will be using descriptors.
Thank you Mark.
Cheers,
-Paul
Hi Paul,
On 2022/4/20 上午1:13, Paul Cercueil wrote:
> Hi Zhou,
>
> Le ven., avril 15 2022 at 22:22:06 +0800, 周琰杰 (Zhou Yanjie)
> <[email protected]> a écrit :
>> Add support for using GPIOs as chip select lines on Ingenic SoCs.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <[email protected]>
>> ---
>> drivers/spi/spi-ingenic.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/spi/spi-ingenic.c b/drivers/spi/spi-ingenic.c
>> index 03077a7..672e4ed 100644
>> --- a/drivers/spi/spi-ingenic.c
>> +++ b/drivers/spi/spi-ingenic.c
>> @@ -380,7 +380,7 @@ static int spi_ingenic_probe(struct
>> platform_device *pdev)
>> struct spi_controller *ctlr;
>> struct ingenic_spi *priv;
>> void __iomem *base;
>> - int ret;
>> + int num_cs, ret;
>>
>> pdata = of_device_get_match_data(dev);
>> if (!pdata) {
>> @@ -416,6 +416,11 @@ static int spi_ingenic_probe(struct
>> platform_device *pdev)
>> if (IS_ERR(priv->flen_field))
>> return PTR_ERR(priv->flen_field);
>>
>> + if (of_property_read_u32(dev->of_node, "num-cs", &num_cs)) {
>
> One small comment here - I think it would be better to use
> device_property_read_u32().
>
> The driver should also use device_get_match_data() instead of
> of_device_get_match_data(), but that's a cleanup that can be done later.
>
Sure, I will send v2.
Thanks and best regards!
> Cheers,
> -Paul
>
>> + dev_warn(dev, "Number of chip select lines not specified.\n");
>> + num_cs = 2;
>> + }
>> +
>> platform_set_drvdata(pdev, ctlr);
>>
>> ctlr->prepare_transfer_hardware = spi_ingenic_prepare_hardware;
>> @@ -429,7 +434,9 @@ static int spi_ingenic_probe(struct
>> platform_device *pdev)
>> ctlr->bits_per_word_mask = pdata->bits_per_word_mask;
>> ctlr->min_speed_hz = 7200;
>> ctlr->max_speed_hz = 54000000;
>> - ctlr->num_chipselect = 2;
>> + ctlr->use_gpio_descriptors = true;
>> + ctlr->max_native_cs = 2;
>> + ctlr->num_chipselect = num_cs;
>> ctlr->dev.of_node = pdev->dev.of_node;
>>
>> if (spi_ingenic_request_dma(ctlr, dev))
>> --
>> 2.7.4
>>
>
Hi Mark,
On 2022/4/20 上午1:00, Mark Brown wrote:
> On Sat, Apr 16, 2022 at 05:36:46PM +0100, Paul Cercueil wrote:
>> Le sam., avril 16 2022 at 19:55:13 +0800, Zhou Yanjie
>>> On 2022/4/15 下午11:00, Paul Cercueil wrote:
>>>>> - ctlr->num_chipselect = 2;
>>>>> + ctlr->use_gpio_descriptors = true;
>>>> I wonder if this should be set conditionally instead. Maybe set it
>>>> to "true" if the "num-cs" property exists?
>>> I'm not too sure, but it seems some other drivers like "spi-sun6i.c",
>>> "spi-stm32.c", "spi-s3c64xx.c", "spi-pic32.c", etc. set it
>>> unconditionally.
>> Ok, maybe Mark can enlighten us here.
> use_gpio_descriptions is just selecting which version of the GPIO APIs
> we should use if we're handling GPIOs rather than if we should handle
> them. We've got one last driver using the numerical GPIO APIs, once
> that one is converted we should just be able to drop the flag since
> everything will be using descriptors.
Thanks for your answer!