On 24/10/2019 15:57, Gregory CLEMENT wrote:
> Hello Jon,
>
>> On 18/10/2019 16:29, Gregory CLEMENT wrote:
>>> When improving the CS GPIO support at core level, the SPI_CS_HIGH
>>> has been enabled for all the CS lines used for a given SPI controller.
>>>
>>> However, the SPI framework allows to have on the same controller native
>>> CS and GPIO CS. The native CS may not support the SPI_CS_HIGH, so they
>>> should not be setup automatically.
>>>
>>> With this patch the setting is done only for the CS that will use a
>>> GPIO as CS
>>>
>>> Fixes: f3186dd87669 ("spi: Optionally use GPIO descriptors for CS GPIOs")
>>> Cc: <stable@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxx>
>>> ---
>>> drivers/spi/spi.c | 18 +++++++++---------
>>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>> index 5414a10afd65..1b68acc28c8f 100644
>>> --- a/drivers/spi/spi.c
>>> +++ b/drivers/spi/spi.c
>>> @@ -1880,15 +1880,7 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>>> spi->mode |= SPI_3WIRE;
>>> if (of_property_read_bool(nc, "spi-lsb-first"))
>>> spi->mode |= SPI_LSB_FIRST;
>>> -
>>> - /*
>>> - * For descriptors associated with the device, polarity inversion is
>>> - * handled in the gpiolib, so all chip selects are "active high" in
>>> - * the logical sense, the gpiolib will invert the line if need be.
>>> - */
>>> - if (ctlr->use_gpio_descriptors)
>>> - spi->mode |= SPI_CS_HIGH;
>>> - else if (of_property_read_bool(nc, "spi-cs-high"))
>>> + if (of_property_read_bool(nc, "spi-cs-high"))
>>> spi->mode |= SPI_CS_HIGH;
>>>
>>> /* Device DUAL/QUAD mode */
>>> @@ -1952,6 +1944,14 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>>> }
>>> spi->chip_select = value;
>>>
>>> + /*
>>> + * For descriptors associated with the device, polarity inversion is
>>> + * handled in the gpiolib, so all gpio chip selects are "active high"
>>> + * in the logical sense, the gpiolib will invert the line if need be.
>>> + */
>>> + if ((ctlr->use_gpio_descriptors) && ctlr->cs_gpiods[spi->chip_select])
>>> + spi->mode |= SPI_CS_HIGH;
>>> +
>>
>> This patch is causing a boot regression on one of our Tegra boards.
>> Bisect is pointing to this commit and reverting on top of today's -next
>> fixes the problem.
>>
>> This patch is causing the following NULL pointer crash which I assume is
>> because we have not checked if 'ctlr->cs_gpiods' is valid before
>> dereferencing ...
>
> I've just submitted a fixe for it
>
> https://patchwork.kernel.org/patch/11209839/
Great! Thanks, Jon
--
nvpublic
