On Thu, Nov 17, 2016 at 06:40:17PM +0100, Mike Looijmans wrote:
ïOn 17-11-16 17:56, Guenter Roeck wrote:[ ... ]
On 11/17/2016 04:10 AM, Tom Levens wrote:
Updated version of the ltc2990 driver which supports all measurement
modes available in the chip. The mode can be set through a devicetree
attribute.
Unlikely but possible. Even if we all agree that the chip should be configured>from the chip instead if it isn't provided (after all, it may have been
static int ltc2990_i2c_probe(struct i2c_client *i2c,
const struct i2c_device_id *id)
{
int ret;
struct device *hwmon_dev;
+ struct ltc2990_data *data;
+ struct device_node *of_node = i2c->dev.of_node;
if (!i2c_check_functionality(i2c->adapter,
I2C_FUNC_SMBUS_BYTE_DATA |
I2C_FUNC_SMBUS_WORD_DATA))
return -ENODEV;
- /* Setup continuous mode, current monitor */
+ data = devm_kzalloc(&i2c->dev, sizeof(struct ltc2990_data),
GFP_KERNEL);
+ if (unlikely(!data))
+ return -ENOMEM;
+ data->i2c = i2c;
+
+ if (!of_node || of_property_read_u32(of_node, "lltc,mode",
&data->mode))
+ data->mode = LTC2990_CONTROL_MODE_DEFAULT;
Iam arguing with myself if we should still do this or if we should read
the mode
initialized
by the BIOS/ROMMON).
I think the mode should be explicitly set, without default. There's no way
to tell whether the BIOS or bootloader has really set it up or whether the
chip is just reporting whatever it happened to default to. And given the
chip's function, it's unlikely a bootloader would want to initialize it.
by the driver, I don't like imposing that view on everyone else.
My advice would be to make it a required property. If not set, display anIt is not that easy, unfortunately. It also has to work on a non-devicetree
error and bail out.
system. I would not object to making the property mandatory, but we would
still need to provide non-DT support.
My "use case" for taking the current mode from the chip if not specified
is that it would enable me to run a module test with all modes. I consider
this extremely valuable.
I should have asked if your system uses devicetree. If it does, the problemMike, would that break your application, or can you specify the mode in
devicetree ?
I'm fine with specifying this in the devicetree. It will break things for
me, but I've been warned and willing to bow for the greater good :)
should be easy to fix for you. If not, we'll need to find a solution
for your use case.
Thanks,
Guenter