Re: Re: [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices

Re: Re: [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices

[MyungJoo Ham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 2681 bytes --]

Mark Brown wrote:
> On Tue, Feb 14, 2012 at 11:22:14AM +0900, MyungJoo Ham wrote:
> > On Sat, Feb 11, 2012 at 1:25 AM, Mark Brown
> > <broonie@opensource.wolfsonmicro.com> wrote:
> > > On Fri, Feb 10, 2012 at 03:40:38PM +0900, MyungJoo Ham wrote:
> > >> External connector devices that decides connection information based on
> > >> ADC values may use adc-jack device driver. The user simply needs to
> > >> provide a table of adc range and connection states. Then, extcon
> > >> framework will automatically notify others.
> 
> > > This really should be done in terms of the IIO in-kernel framework.
> 
> > The ADC part may be done in IIO. However, the intention of this device
> > driver is to provide extcon interface to any ADC drivers, not
> > providing an ADC device driver. If we are going to implement this in
> 
> Right, exactly.
> 
> > the ADC driver in IIO, we will need to write the given code in every
> > ADC driver used for analog ports.
> 
> No, that's not what I'm suggesting - what I'm suggesting is that rather
> than having a callback for implementing the ADC read functionality this
> should work as an in-kernel IIO driver so it'll just automatically work
> with any ADC without needing code to hook things up.  Unless I've not
> understood your comment fully.

Ok. Do you mean that we should give struct iio_dev * and struct iio_chan_spec * instead of a callback?
I thought there was no standard framework for ADC (the reason why this is a simple callback); however, may I regard iio/adc as the standard for ADC? (I see some ADC device drivers in HWMON)

Do I need to make this driver as an iio device driver in order to do this? I guess including iio.h would be enough anyway.


> 
> > >> +     /* Check the length of array and set num_cables */
> > >> +     for (i = 0; data->edev.supported_cable[i]; i++)
> > >> +             ;
> > >> +     if (i == 0 || i > SUPPORTED_CABLE_MAX) {
> 
> > > Can we not avoid the hard limit?
> 
> > Without that limit, we won't be able to easily express binary cable
> > status (u32) with the extcon framework. At least, we will need to
> > forget about setting the status with u32 values.
> 
> > Anyway, I can remove the checking SUPPORT_CABLE_MAX part at probe.
> 
> It might be clearer to make the limit more obviously associated with
> the bitmask - it looks like it's an array thing the way the code is
> written but a limit due to using a bitmask seems reasonable.
> 
> 

Ah.. I'll add a comment or update the error message associated.


Cheers!
MyungJoo.

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: Re: [PATCH v5 5/5] Extcon: adc-jack driver to support 3.5 pi or simliar devices

[Mark Brown]

On Tue, Feb 14, 2012 at 07:00:48AM +0000, MyungJoo Ham wrote:
> Mark Brown wrote:

> > No, that's not what I'm suggesting - what I'm suggesting is that rather
> > than having a callback for implementing the ADC read functionality this
> > should work as an in-kernel IIO driver so it'll just automatically work
> > with any ADC without needing code to hook things up.  Unless I've not
> > understood your comment fully.

> Ok. Do you mean that we should give struct iio_dev * and struct
> iio_chan_spec * instead of a callback?  I thought there was no
> standard framework for ADC (the reason why this is a simple callback);
> however, may I regard iio/adc as the standard for ADC? (I see some ADC
> device drivers in HWMON)

Yes, the idea is that IIO becomes the standard way of using general
purpose ADCs like this in the kernel rather than having lots of similar
custom APIs floating around with callbacks as we do now.

> Do I need to make this driver as an iio device driver in order to do this? I guess including iio.h would be enough anyway.

I'd not expect so, I'd expect it's just a user of IIO.