[PATCH] iio:dac:ad5755 fixed SPI_MODE

[PATCH] iio:dac:ad5755 fixed SPI_MODE

[Sean Nyekjaer]

According to datasheet data is latched on the falling edge of the SCK.

I agree it works in SPI_MODE_0 on the eval kit, but in other configurations
with optic isolators (that adds a small delay). We are falling out of spec.

Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
---
 drivers/iio/dac/ad5755.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/dac/ad5755.c b/drivers/iio/dac/ad5755.c
index bfb350a..189d29a 100644
--- a/drivers/iio/dac/ad5755.c
+++ b/drivers/iio/dac/ad5755.c
@@ -573,6 +573,8 @@ static int ad5755_probe(struct spi_device *spi)
 	st = iio_priv(indio_dev);
 	spi_set_drvdata(spi, indio_dev);
 
+	spi->mode = SPI_MODE_1;
+
 	st->chip_info = &ad5755_chip_info_tbl[type];
 	st->spi = spi;
 	st->pwr_down = 0xf;
-- 
2.7.0

Re: [PATCH] iio:dac:ad5755 fixed SPI_MODE

[Lars-Peter Clausen]

On 01/27/2016 03:39 PM, Sean Nyekjaer wrote:
> According to datasheet data is latched on the falling edge of the SCK.
> 
> I agree it works in SPI_MODE_0 on the eval kit, but in other configurations
> with optic isolators (that adds a small delay). We are falling out of spec.
> 
> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>

Hi,

The SPI mode is usually specified in the spi_board_info struct or devicetree
rather than hardcoding it in the driver.

- Lars

Re: [PATCH] iio:dac:ad5755 fixed SPI_MODE

[Sean Nyekjær]

On 2016-01-27 15:45, Lars-Peter Clausen wrote:
> On 01/27/2016 03:39 PM, Sean Nyekjaer wrote:
>> According to datasheet data is latched on the falling edge of the SCK.
>>
>> I agree it works in SPI_MODE_0 on the eval kit, but in other configurations
>> with optic isolators (that adds a small delay). We are falling out of spec.
>>
>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
> Hi,
>
> The SPI mode is usually specified in the spi_board_info struct or devicetree
> rather than hardcoding it in the driver.
>
> - Lars
Hi
I know :-)
But, all devices in this family have only support for SPI_MODE_1. So why 
leave it up to the spi_board_info or devicetree when only one mode is 
allowed?
If you don't like this patch I think we should create some documentation 
for the device, so other people don't have to use hours in debugging :-D

/Sean
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] iio:dac:ad5755 fixed SPI_MODE

[Lars-Peter Clausen]

On 01/27/2016 04:33 PM, Sean Nyekjær wrote:
> 
> On 2016-01-27 15:45, Lars-Peter Clausen wrote:
>> On 01/27/2016 03:39 PM, Sean Nyekjaer wrote:
>>> According to datasheet data is latched on the falling edge of the SCK.
>>>
>>> I agree it works in SPI_MODE_0 on the eval kit, but in other configurations
>>> with optic isolators (that adds a small delay). We are falling out of spec.
>>>
>>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
>> Hi,
>>
>> The SPI mode is usually specified in the spi_board_info struct or devicetree
>> rather than hardcoding it in the driver.
>>
>> - Lars
> Hi
> I know :-)
> But, all devices in this family have only support for SPI_MODE_1. So why
> leave it up to the spi_board_info or devicetree when only one mode is allowed?

As far as I can see they should work with either SPI_MODE_1 or SPI_MODE_2.
But it would certainly be nice if the SPI framework had support for
auto-configuration, based on the support modes of the SPI controller and the
device, in case no mode was explicit specified.

> If you don't like this patch I think we should create some documentation for
> the device, so other people don't have to use hours in debugging :-D

https://wiki.analog.com/resources/tools-software/linux-drivers/iio-dac/ad5755?&#example_platform_device_initialization

Re: [PATCH] iio:dac:ad5755 fixed SPI_MODE

[Sean Nyekjær]

On 2016-01-27 16:45, Lars-Peter Clausen wrote:
> On 01/27/2016 04:33 PM, Sean Nyekjær wrote:
>> On 2016-01-27 15:45, Lars-Peter Clausen wrote:
>>> On 01/27/2016 03:39 PM, Sean Nyekjaer wrote:
>>>> According to datasheet data is latched on the falling edge of the SCK.
>>>>
>>>> I agree it works in SPI_MODE_0 on the eval kit, but in other configurations
>>>> with optic isolators (that adds a small delay). We are falling out of spec.
>>>>
>>>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
>>> Hi,
>>>
>>> The SPI mode is usually specified in the spi_board_info struct or devicetree
>>> rather than hardcoding it in the driver.
>>>
>>> - Lars
>> Hi
>> I know :-)
>> But, all devices in this family have only support for SPI_MODE_1. So why
>> leave it up to the spi_board_info or devicetree when only one mode is allowed?
> As far as I can see they should work with either SPI_MODE_1 or SPI_MODE_2.
> But it would certainly be nice if the SPI framework had support for
> auto-configuration, based on the support modes of the SPI controller and the
> device, in case no mode was explicit specified.
>
>> If you don't like this patch I think we should create some documentation for
>> the device, so other people don't have to use hours in debugging :-D
> https://wiki.analog.com/resources/tools-software/linux-drivers/iio-dac/ad5755?&#example_platform_device_initialization
Ops the gmail app on android is not very good answering text only emails :-)

Is there a reason why this documentation is not in the kernel?
It's also missing the options and examples for devicetree bindings...

Should I create the devicetree documentation instead of this patch?
I still think this patch is a better solution to prevent future problems 
with the driver.

/Sean

Re: [PATCH] iio:dac:ad5755 fixed SPI_MODE

[Lars-Peter Clausen]

On 01/28/2016 08:29 AM, Sean Nyekjær wrote:
> 
> 
> On 2016-01-27 16:45, Lars-Peter Clausen wrote:
>> On 01/27/2016 04:33 PM, Sean Nyekjær wrote:
>>> On 2016-01-27 15:45, Lars-Peter Clausen wrote:
>>>> On 01/27/2016 03:39 PM, Sean Nyekjaer wrote:
>>>>> According to datasheet data is latched on the falling edge of the SCK.
>>>>>
>>>>> I agree it works in SPI_MODE_0 on the eval kit, but in other
>>>>> configurations
>>>>> with optic isolators (that adds a small delay). We are falling out of
>>>>> spec.
>>>>>
>>>>> Signed-off-by: Sean Nyekjaer <sean.nyekjaer@prevas.dk>
>>>> Hi,
>>>>
>>>> The SPI mode is usually specified in the spi_board_info struct or
>>>> devicetree
>>>> rather than hardcoding it in the driver.
>>>>
>>>> - Lars
>>> Hi
>>> I know :-)
>>> But, all devices in this family have only support for SPI_MODE_1. So why
>>> leave it up to the spi_board_info or devicetree when only one mode is
>>> allowed?
>> As far as I can see they should work with either SPI_MODE_1 or SPI_MODE_2.
>> But it would certainly be nice if the SPI framework had support for
>> auto-configuration, based on the support modes of the SPI controller and the
>> device, in case no mode was explicit specified.
>>
>>> If you don't like this patch I think we should create some documentation for
>>> the device, so other people don't have to use hours in debugging :-D
>> https://wiki.analog.com/resources/tools-software/linux-drivers/iio-dac/ad5755?&#example_platform_device_initialization
>>
> Ops the gmail app on android is not very good answering text only emails :-)
> 
> Is there a reason why this documentation is not in the kernel?

It might make sense to add a subset of this to the kernel repository itself.
But I'm not convinced that the kernel tree is the best place to host this
kind of extended documentation.

> It's also missing the options and examples for devicetree bindings...

There is no real devicetree support in the driver yet. You can use the
driver, but it will only work with the default pdata, which is not want you
want in a lot of cases. There is also not compatible string for the device.

> 
> Should I create the devicetree documentation instead of this patch?
> I still think this patch is a better solution to prevent future problems
> with the driver.

Sounds good, thanks.

- Lars