RE: [PATCH 0/4] staging:iio:meter clean up and dead code removal.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jonathan Cameron wrote on 2011-02-07:
> On 02/07/11 15:34, Hennerich, Michael wrote:
>> Jonathan Cameron wrote on 2011-02-05:
>>> This is very much the starting point for bashing the meter drivers
>>> into shape.  For now I've just removed most of the unused code and
> stubs.
>>
>> Instead of removing the stubs - I would rather prefer implementing
> them.
> I agree in the ideal case.  But even then, it is much easier to review
> a patch that adds this functionality from scratch than it is to review
> one that relies on large amounts of existing dead code.
>> But I think you're right this dead code is not going to be useful
>> either.
>>
>>> I have also cleaned up spi bus handling.  I may well have gotten that
>>> bit wrong so please do take a look.  In some cases this involved
>>> turning one transfer into two so as to clarify what the code was
>>> doing. This may cause issues if the spi bus drivers don't play the
>>> game properly or as if I got the conversion wrong.
>>
>> Looks right - but I had to double check with the datasheet. The SPI
>> write then read behavior is a bit undefined. In particular, what is
>> shifted out on the MOSI pin during the receive part.
>>
>> Some implementations send zero/one or repeat the last word in the shift
>> register, while others tristate MOSI. If the target slave device in
>> questions also listens on MOSI while it drives MISO, unpredicted
>> behavior my result.
> Strange, I thought this was defined as always sending zero if not
> provided.  If not then I would suggest we point this inconsistency out
> to the spi guys and fix it up in those implementations that don't
> conform.  Otherwise as you say, one needs to always specify the other
> direction just to make sure nothing weird happens.
> All my testing is on ancient pxa boards, so I know that they always
> send zeros and dump whatever is recieved.

This behavior is implemented in the SPI Master Controller Drivers, and not in the SPI Bus Core.
I know in the past some machine dependant implementations did it wrong.
However looking at the drivers today they all (at least the ones I looked at)
send zero in case tx_buf is NULL.

>>
>> It's therefore sometimes necessary to use full duplex transfer and
>> drive MOSI to a known good state. You basically removed this, but
>> the ADExxxx Parts don't seem to depend on this.
>>
>>> Next job is to pin down, standardise and document the sysfs interface
>>> for these parts.  Anyone care to take this on?
>>
>> Considering the complexity and number of new arguments, this is going
>> to take some time. But it has to be done. Unfortunately right now I'm a
>> bit loaded with other stuff. But I'll open a tracker item for me to
>> look at.
> Cool.  If I get really board I might do a data sheet trawl and try and
> pin this down in the meantime.

Hope you get board anytime soon. :-)
No just kidding. If you want to take the lead here,feel free, and I also can get you
an evaluation board. But I don't expect you to work on this.

>>
>>> Note I have currently left the ade7758 driver alone as it is by far
>>> the most complex and didn't suffer the large amount of dead code to
>>> be found in the others.
>>>
>>> All comments as ever welcome.  I'd also like to know what test
>>> coverage we have on these parts.
>>
>> The ADE7758 is the only one I have test hardware. Ideally we bring
>> this one in a good shape and add support for the other parts to it.
> Works for me.  Though they are pretty different so I'm not sure a
> unified driver would work terribly well for these.  Could be wrong
> though :)

Folding all in one driver probably get's a bit messy. Whatever makes the
best sense, while preserving readability and maintainability.

Thanks,
Michael


>>
>>> Thanks,
>>>
>>> Jonathan
>>>
>>> Jonathan Cameron (4):
>>>   staging:iio:meter remove stubs from ade7753.
>>>   staging:iio:meter remove stubs from ade7754.
>>>   staging:iio:meter remove stubs from ade7759.
>>>   staging:iio:meter remove stubs from ade7854.
>>  drivers/staging/iio/meter/ade7753.c     |  200 +++++++--------------

Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368; Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif


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


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux