pca9665

pca9665

[Marco Aurelio da Costa]

Hi Wolfram.

I looked into the code for i2c-algo-pca.c and, if I don't want to 
implement the buffered mode (I can live without it for a while), the 
only changes I need to do are:
1. Implement the reset via parallel bus.
2. Change the way the i2c clock rate is treated, by passing the 
frequency directly instead of an index.

After this, this module could handle both chips. I need your advice:
Should I just duplicate all code and make the changes to this new algo 
or should I add a chip_type field to i2c_algo_pca_data and handle the 
chip on the existing algo code?

Thanks in advance,

Marco

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: pca9665

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 1634 bytes --]

Hello Marco,

sorry for the late reply, I had to work on other parts first.

On Wed, Sep 17, 2008 at 12:30:29PM -0300, Marco Aurelio da Costa wrote:

> I looked into the code for i2c-algo-pca.c and, if I don't want to 
> implement the buffered mode (I can live without it for a while),

But it may be worth to already get a rough idea if the buffered mode
will fit into this algorithm at all.

> only changes I need to do are:
> 1. Implement the reset via parallel bus.

I can't see this point right now. The algorithm expects a reset routine
from the driver using the algorithm; this should be flexible enough, no?
Or do you mean you also want to share i2c-pca-platform.c among those two
chips. Then it's correct to change it there. Otherwise, please enlighten
me, it's been some time :)

> 2. Change the way the i2c clock rate is treated, by passing the 
> frequency directly instead of an index.

Yup, this has to be generalized between these two.

> After this, this module could handle both chips. I need your advice:
> Should I just duplicate all code and make the changes to this new algo 
> or should I add a chip_type field to i2c_algo_pca_data and handle the 
> chip on the existing algo code?

Concerning the algorithm, I'd really go for the latter. It is easier to
maintain if there will be bugfixes to the state-machine, for example.
And they seem to be similar enough (perhaps modulo the buffered mode,
which I didn't look at so far).

All the best,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: pca9665

[Marco Aurelio da Costa]

Hi Wolfram,


Wolfram Sang wrote:
> Hello Marco,
>
> sorry for the late reply, I had to work on other parts first.
>   
No problem.
> On Wed, Sep 17, 2008 at 12:30:29PM -0300, Marco Aurelio da Costa wrote:
>
>   
>> I looked into the code for i2c-algo-pca.c and, if I don't want to 
>> implement the buffered mode (I can live without it for a while),
>>     
>
> But it may be worth to already get a rough idea if the buffered mode
> will fit into this algorithm at all.
>   
I will have to understand the buffered mode first, but at worse I will 
have to check which mode is being used and add another logic path.
>   
>> only changes I need to do are:
>> 1. Implement the reset via parallel bus.
>>     
>
> I can't see this point right now. The algorithm expects a reset routine
> from the driver using the algorithm; this should be flexible enough, no?
> Or do you mean you also want to share i2c-pca-platform.c among those two
> chips. Then it's correct to change it there. Otherwise, please enlighten
> me, it's been some time :)
>   
My ideas was to share the platform and the isa code. This is easy 
enough. For the PCA9665, the platform will not define the reset 
function, and the algorithm will not even try use it.
>   
>> 2. Change the way the i2c clock rate is treated, by passing the 
>> frequency directly instead of an index.
>>     
>
> Yup, this has to be generalized between these two.
>   
I can do that then. We should then pass the frequency. For the 9564, I 
will just accept the chip defined frequencies.
>   
>> After this, this module could handle both chips. I need your advice:
>> Should I just duplicate all code and make the changes to this new algo 
>> or should I add a chip_type field to i2c_algo_pca_data and handle the 
>> chip on the existing algo code?
>>     
>
> Concerning the algorithm, I'd really go for the latter. It is easier to
> maintain if there will be bugfixes to the state-machine, for example.
> And they seem to be similar enough (perhaps modulo the buffered mode,
> which I didn't look at so far).
>   
That is also my view. I will start coding, and will send you the patch 
for review later.
> All the best,
>
>    Wolfram
>
>   
Regards,

Marco


_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: pca9665

[Wolfram Sang]

[-- Attachment #1.1: Type: text/plain, Size: 1114 bytes --]

Hi Marco,

On Fri, Sep 19, 2008 at 08:37:25AM -0300, Marco Aurelio da Costa wrote:

> I will have to understand the buffered mode first, but at worse I will 
> have to check which mode is being used and add another logic path.

Yes, let's hope it can be worked into the current logic then...

> My ideas was to share the platform and the isa code. This is easy 
> enough. For the PCA9665, the platform will not define the reset 
> function, and the algorithm will not even try use it.

Okay, got it. I agree.

> I can do that then. We should then pass the frequency. For the 9564, I 
> will just accept the chip defined frequencies.

Sounds like the right way to do it. I just wonder if we simply can
enforce this change of platform_data or if we should provide some
transition feature. Jean, Ben, others, what are your experiences?

> That is also my view. I will start coding, and will send you the patch 
> for review later.

Happy hacking! :D

Bye,

   Wolfram

-- 
  Dipl.-Ing. Wolfram Sang | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: pca9665

[Marco Aurelio da Costa]

Wolfram Sang wrote:
>
> Sounds like the right way to do it. I just wonder if we simply can
> enforce this change of platform_data or if we should provide some
> transition feature. Jean, Ben, others, what are your experiences?
>
>   
The way it does today is to pass an index for the frequency.
I can make the change so that it uses the 9564 by default, and if the 
frequency is set from 0 - 7 (not possible for a real frequency), I will 
use it as an index. We may put a warning that this usage is deprecated. 
Is this acceptable?

Marco

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c

Re: pca9665

[Jean Delvare]

On Fri, 19 Sep 2008 09:09:53 -0300, Marco Aurelio da Costa wrote:
> 
> Wolfram Sang wrote:
> >
> > Sounds like the right way to do it. I just wonder if we simply can
> > enforce this change of platform_data or if we should provide some
> > transition feature. Jean, Ben, others, what are your experiences?
>
> The way it does today is to pass an index for the frequency.
> I can make the change so that it uses the 9564 by default, and if the 
> frequency is set from 0 - 7 (not possible for a real frequency), I will 
> use it as an index. We may put a warning that this usage is deprecated. 
> Is this acceptable?

Sounds OK to me.

-- 
Jean Delvare

_______________________________________________
i2c mailing list
i2c-GZX6beZjE8VD60Wz+7aTrA@public.gmane.org
http://lists.lm-sensors.org/mailman/listinfo/i2c