On 07/05/2014 01:53 AM, Jean Delvare wrote:
Hi Guenter,
On Fri, 04 Jul 2014 18:01:25 -0700, Guenter Roeck wrote:
On 07/04/2014 01:20 PM, Jean Delvare wrote:
Not just "this is a bank register". You also most specify which bits
set the bank number (which in turn define the maximum number of banks),
and the range of banked registers.
I did not think about it in depth yet, but there are two options that
come to my mind: module parameters indeed, or sysfs attributes. We
could create a sysfs directory for every client address and have
writable attributes bank_reg, bank_mask, banked_start_reg and
banked_end_reg in that directory. Not sure exactly where actually,
maybe we would need to introduce a virtual i2c-stub device, or use
debugfs.
Module parameters would work too, that's probably easier to implement
that way, but that's also less flexible, as you have to setup
everything upon module loading. That might be just fine though, as
being able to change the bank setup at run-time has little practical
value IMHO.
At least with my module test scripts, I re-load the i2c-stub driver
before testing a new chip, to make sure that its state is 'clean'.
So module parameters work just fine for me.
I started working on the idea using module parameters. A lot more work
is needed though, and I have a lot of other things to do today.
Could we use a single module parameter ?
bank_reg=<reg> <mask>
What would be the benefit? Using parameters with single integer values
is simple. Using space-separated lists would require us to parse the
parameters ourselves.
Sorry, I meant using a parameter array.
would probably do for a start, and mask could even be optional.
I don't think we can make the mask optional. For one thing, that would
force us to allocate memory for all 256 possible banks, which would be
a waste of memory. For another, it would make a single bank appear as
separate ones, so changes to one of the bank "copies" would not reflect
in the other "copies". Winbond for example uses the MSB of the register
bank to select the low or high byte of the vendor ID.
Not sure
if banked_start_reg and banked_end_reg are necessary. If so the above
could be extended to
bank_reg=<reg> <mask> <start> <end>
I think we have to, for mostly the same reason we need to limit the
number of banks: non-banked registers are shared between banks. If we
don't specify the range of banked registers, changes done to shared
registers won't reflect between banks, and that will confuse the
drivers.
Ok, makes sense.
One thing I'm not yet sure of is if we want to handle banks on more
than one chip at a time. I started coding that way but it might be
somewhat overkill actually. I guess you won't need that for your
testing.
No, I would consider that overkill.
That would not work for PMBus chips - those are so complex that
it is unlikely we'll ever be able to simulate one without much
more complexity - it would require the ability to return a failure
when trying to access unsupported registers/commands, and that
on each of the pages.
PMBus chips are a breed on their own. I don't think the generic
i2c-stub driver will ever be enough for these. You'd need a dedicated
emulator for them.
Agreed.
Something else though that would help: SMBus block commands.
Any idea why this is not currently supported ? I see that
I2C_SMBUS_I2C_BLOCK_DATA is supported, so I2C_SMBUS_BLOCK_DATA
should not be hard. Am I missing something ?
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
