pca9665

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),

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?

Hello Marco,

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

Quoted 5 lines

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.

Quoted 4 lines

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 :)

Quoted 4 lines

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.

Quoted 6 lines

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

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.

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.

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

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

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?

Quoted 4 lines · Wolfram Sang

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?