From: javier.martinez@collabora•co.uk (Javier Martinez Canillas)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/2 v3] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio"
Date: Wed, 11 Jun 2014 20:18:20 +0200 [thread overview]
Message-ID: <53989D6C.5080806@collabora.co.uk> (raw)
In-Reply-To: <539896FB.4050702@gmail.com>
Hello Tomasz,
On 06/11/2014 07:50 PM, Tomasz Figa wrote:
> On 11.06.2014 19:27, Javier Martinez Canillas wrote:
>> On 06/11/2014 01:38 PM, Naveen Krishna Ch wrote:
>>> On 11 June 2014 16:43, Javier Martinez Canillas
>>> <javier.martinez@collabora•co.uk> wrote:
>>>> On 06/11/2014 08:31 AM, Naveen Krishna Chatradhi wrote:
>
> [snip]
>
>>>>
>>>>> return ERR_PTR(-EINVAL);
>>>>> }
>>>>> + cs->line = spi->cs_gpio;
>>>>>
>>>>
>>>> I wonder why are you keeping cs->line? AFAICT it's only used in
>>>> s3c64xx_spi_setup() to request the GPIO and since you get the struct spi_device
>>>> pointer as a parameter then you can just use spi->s_gpio instead.
>>> I'm trying not to touch the non-DT part of the code.
>>>
>>> struct s3c64xx_spi_csinfo *cs = spi->controller_data;
>>>
>>> This will update the cs->line and cs->fb_delay in case of non-DT.
>>
>> I see, then I prefer the opposite and do something like this on s3c64xx_spi_probe():
>>
>> if (!pdev->dev.of_node)
>> spi->cs_gpio = cs->line;
>
> Hmm, as far as I understand, spi here is spi_device, not spi_master. I
> don't think you have access to SPI devices on your bus in controller
> probe().
>
Right, I was actually looking at s3c64xx_spi_setup() when I wrote that but for
some reason I got confused and thought it was the probe() function.
Sorry for the confusion.
> What I think could work is reworking the driver to:
>
> - in DT case, don't do anything in the driver about the GPIO chip
> select, because it will be handled automatically by the core.
>
> - in non-DT case, in s3c64xx_spi_setup(), just take the GPIO pin from
> s3c64xx_spi_csinfo struct passed through spi->controller_data, request
> it and save it to spi->cs_gpio,
>
> - in non-DT case, in s3c64xx_spi_cleanup(), free the GPIO requested in
> s3c64xx_spi_setup() and set spi->cs_gpio to -ENOENT (as done initially
> in spi_alloc_device()).
>
> Best regards,
> Tomasz
>
Best regards,
Javier
next prev parent reply other threads:[~2014-06-11 18:18 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-11 6:31 [PATCH 1/2 v3] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio" Naveen Krishna Chatradhi
2014-06-11 6:31 ` [PATCH 2/2 v3] ARM: DTS: move "cs-gpio" from "controller-data" to under spi node Naveen Krishna Chatradhi
2014-06-11 11:21 ` Javier Martinez Canillas
2014-06-11 11:29 ` Naveen Krishna Ch
2014-06-11 11:13 ` [PATCH 1/2 v3] spi: s3c64xx: use "cs-gpios" from spi node instead of "cs-gpio" Javier Martinez Canillas
2014-06-11 11:38 ` Naveen Krishna Ch
2014-06-11 17:27 ` Javier Martinez Canillas
2014-06-11 17:50 ` Tomasz Figa
2014-06-11 18:18 ` Javier Martinez Canillas [this message]
2014-06-11 18:23 ` Naveen Krishna Ch
2014-06-11 18:27 ` Mark Brown
2014-06-11 18:28 ` Tomasz Figa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53989D6C.5080806@collabora.co.uk \
--to=javier.martinez@collabora$(echo .)co.uk \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox