diff mbox series

[v3,3/3] i2c/busses/i2c-icy: Add LTC2990 present on 2019 board revision

Message ID 20190815125802.16500-3-max@enpas.org (mailing list archive)
State Not Applicable
Headers show
Series [v3,1/3] i2c/busses: Add i2c-icy for I2C on m68k/Amiga | expand

Commit Message

Max Staudt Aug. 15, 2019, 12:58 p.m. UTC
Since the 2019 a1k.org community re-print of these PCBs sports an
LTC2990 hwmon chip as an example use case, let this driver autoprobe
for that as well. If it is present, modprobing ltc2990 is sufficient.

The property_entry enables the three additional inputs available on
this particular board:

  in1 will be the voltage of the 5V rail, divided by 2.
  in2 will be the voltage of the 12V rail, divided by 4.
  temp3 will be measured using a PCB loop next the chip.

v3: Merged with initial LTC2990 support on ICY.
    Moved defaults from platform_data to swnode.
    Added note to Kconfig.

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/i2c/busses/Kconfig   |  3 +++
 drivers/i2c/busses/i2c-icy.c | 56 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

Comments

Wolfram Sang Aug. 16, 2019, 11:51 a.m. UTC | #1
> +	if (IS_ERR(new_fwnode))
> +		dev_info(&z->dev, "Failed to create fwnode for LTC2990, error: %ld\n",
> +			 PTR_ERR(new_fwnode));
> +	else {

Braces for both blocks. Did you run checkpatch?

> +		/*
> +		 * Store the fwnode so we can destroy it on .remove().
> +		 * Only store it on success, as fwnode_remove_software_node()
> +		 * is NULL safe, but not PTR_ERR safe.
> +		 */
> +		i2c->ltc2990_fwnode = new_fwnode;
> +		ltc2990_info.fwnode = new_fwnode;
> +
> +		i2c->ltc2990_client =
> +			i2c_new_probed_device(&i2c->adapter,
> +					      &ltc2990_info,
> +					      icy_ltc2990_addresses,
> +					      NULL);

i2c_new_device (or better, the new i2c_new_client_device) should be
sufficient, or? You only have one potential address.
Max Staudt Aug. 16, 2019, 12:34 p.m. UTC | #2
On 08/16/2019 01:51 PM, Wolfram Sang wrote:
> 
>> +	if (IS_ERR(new_fwnode))
>> +		dev_info(&z->dev, "Failed to create fwnode for LTC2990, error: %ld\n",
>> +			 PTR_ERR(new_fwnode));
>> +	else {
> 
> Braces for both blocks. Did you run checkpatch?

I did, and it didn't say anything.

Turns out I misremembered the CodingStyle as having a corner case where it doesn't. I'll fix the style - I dislike the above, too ;)


>> +		/*
>> +		 * Store the fwnode so we can destroy it on .remove().
>> +		 * Only store it on success, as fwnode_remove_software_node()
>> +		 * is NULL safe, but not PTR_ERR safe.
>> +		 */
>> +		i2c->ltc2990_fwnode = new_fwnode;
>> +		ltc2990_info.fwnode = new_fwnode;
>> +
>> +		i2c->ltc2990_client =
>> +			i2c_new_probed_device(&i2c->adapter,
>> +					      &ltc2990_info,
>> +					      icy_ltc2990_addresses,
>> +					      NULL);
> 
> i2c_new_device (or better, the new i2c_new_client_device) should be
> sufficient, or? You only have one potential address.

Yes and no. Now that you mention it - the LTC2990 can be at four addresses (0x4c - 0x4f), and there are jumpers (solder pads) on the PCB to select its address. Shall I add all 4 addresses to the array?

It's also possible that there is no LTC2990 at all (because it's hard to solder at home), and that's why we need to probe for it first. I believe i2c_new[_client]_device doesn't do probing, but rather assumes the device to be there. Correct?


Max
Wolfram Sang Aug. 16, 2019, 4:09 p.m. UTC | #3
> > Braces for both blocks. Did you run checkpatch?
> 
> I did, and it didn't say anything.

Hmm, strange, does is complain when you use '--strict'?

> Turns out I misremembered the CodingStyle as having a corner case
> where it doesn't. I'll fix the style - I dislike the above, too ;)

Thanks!

> Yes and no. Now that you mention it - the LTC2990 can be at four
> addresses (0x4c - 0x4f), and there are jumpers (solder pads) on the
> PCB to select its address. Shall I add all 4 addresses to the array?

Yes, please.

> It's also possible that there is no LTC2990 at all (because it's hard
> to solder at home), and that's why we need to probe for it first. I
> believe i2c_new[_client]_device doesn't do probing, but rather assumes
> the device to be there. Correct?

Correct. I assumed the sensor is always there. But it is not and can be
at multiple addresses, so updating the array of addresses is the right
thing to do(tm).
Max Staudt Aug. 19, 2019, 11:22 a.m. UTC | #4
On 08/16/2019 06:09 PM, Wolfram Sang wrote:
> 
>>> Braces for both blocks. Did you run checkpatch?
>>
>> I did, and it didn't say anything.
> 
> Hmm, strange, does is complain when you use '--strict'?

Yes, --strict catches it. Thanks, I didn't know about that parameter.

I'll send a v5 with all requested fixes.


Max
diff mbox series

Patch

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 9e57e1101..a311d07f3 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1311,6 +1311,9 @@  config I2C_ICY
 	  This support is also available as a module.  If so, the module
 	  will be called i2c-icy.
 
+	  If you have a 2019 edition board with an LTC2990 sensor at address
+	  0x4c, loading the module 'ltc2990' is sufficient to enable it.
+
 config I2C_MLXCPLD
 	tristate "Mellanox I2C driver"
 	depends on X86_64
diff --git a/drivers/i2c/busses/i2c-icy.c b/drivers/i2c/busses/i2c-icy.c
index edac515da..c43cd170f 100644
--- a/drivers/i2c/busses/i2c-icy.c
+++ b/drivers/i2c/busses/i2c-icy.c
@@ -54,6 +54,8 @@  struct icy_i2c {
 
 	void __iomem *reg_s0;
 	void __iomem *reg_s1;
+	struct fwnode_handle *ltc2990_fwnode;
+	struct i2c_client *ltc2990_client;
 };
 
 
@@ -100,11 +102,33 @@  static void icy_pcf_waitforpin(void *data)
 /*
  * Main i2c-icy part
  */
+static unsigned short const icy_ltc2990_addresses[] = {0x4c, I2C_CLIENT_END};
+
+/*
+ * Additional sensors exposed once this property is applied:
+ *
+ * in1 will be the voltage of the 5V rail, divided by 2.
+ * in2 will be the voltage of the 12V rail, divided by 4.
+ * temp3 will be measured using a PCB loop next the chip.
+ */
+static const u32 icy_ltc2990_meas_mode[] = {0, 3};
+
+static const struct property_entry icy_ltc2990_props[] = {
+	PROPERTY_ENTRY_U32_ARRAY("lltc,meas-mode", icy_ltc2990_meas_mode),
+	{ }
+};
+
+
 static int icy_probe(struct zorro_dev *z,
 			 const struct zorro_device_id *ent)
 {
 	struct icy_i2c *i2c;
 	struct i2c_algo_pcf_data *algo_data;
+	struct fwnode_handle *new_fwnode;
+	struct i2c_board_info ltc2990_info = {
+		.type		= "ltc2990",
+		.addr		= 0x4c,
+	};
 
 
 	i2c = devm_kzalloc(&z->dev, sizeof(*i2c), GFP_KERNEL);
@@ -147,6 +171,35 @@  static int icy_probe(struct zorro_dev *z,
 	dev_info(&z->dev, "ICY I2C controller at %pa, IRQ not implemented\n",
 		 &z->resource.start);
 
+	/*
+	 * The 2019 a1k.org PCBs have an LTC2990 at 0x4c, so start
+	 * it automatically once ltc2990 is modprobed.
+	 *
+	 * in0 is the voltage of the internal 5V power supply.
+	 * temp1 is the temperature inside the chip.
+	 *
+	 * See property_entry above for in1, in2, temp3.
+	 */
+	new_fwnode = fwnode_create_software_node(icy_ltc2990_props, NULL);
+	if (IS_ERR(new_fwnode))
+		dev_info(&z->dev, "Failed to create fwnode for LTC2990, error: %ld\n",
+			 PTR_ERR(new_fwnode));
+	else {
+		/*
+		 * Store the fwnode so we can destroy it on .remove().
+		 * Only store it on success, as fwnode_remove_software_node()
+		 * is NULL safe, but not PTR_ERR safe.
+		 */
+		i2c->ltc2990_fwnode = new_fwnode;
+		ltc2990_info.fwnode = new_fwnode;
+
+		i2c->ltc2990_client =
+			i2c_new_probed_device(&i2c->adapter,
+					      &ltc2990_info,
+					      icy_ltc2990_addresses,
+					      NULL);
+	}
+
 	return 0;
 }
 
@@ -154,6 +207,9 @@  static void icy_remove(struct zorro_dev *z)
 {
 	struct icy_i2c *i2c = dev_get_drvdata(&z->dev);
 
+	i2c_unregister_device(i2c->ltc2990_client);
+	fwnode_remove_software_node(i2c->ltc2990_fwnode);
+
 	i2c_del_adapter(&i2c->adapter);
 }