Re: [PATCH v3 2/3] hwmon/ltc2990: Generalise DT to fwnode support
On Fri, Aug 16, 2019 at 12:19:42AM +0200, Max Staudt wrote:
> On 08/15/2019 02:58 PM, Max Staudt wrote:
> > - if (of_node) {
> > - ret = of_property_read_u32_array(of_node, "lltc,meas-mode",
> > -data->mode, 2);
> > + if (i2c->dev.of_node || i2c->dev.fwnode) {
>
> One more idea, would it be better here to do the following?
>
> if (device_property_present(i2c->dev, "lltc,meas-mode")) {
> ret = of_property_read_u32_array(of_node, "lltc,meas-mode",
>data->mode, 2);
> }
>
> I'm happy to prepare a patch if you wish to have this in - just let me know
> whether it should be on top of the last one, or instead of it.
That would be semantically different. The property is currently mandatory.
The above code would make it optional. This might work:
if (dev_fwnode(>dev)) {
ret = device_property_read_u32_array(...);
...
}
Feel free to send another version of your patch.
Thanks,
Guenter
Re: [PATCH v3 2/3] hwmon/ltc2990: Generalise DT to fwnode support
On 08/15/2019 08:33 PM, Guenter Roeck wrote: > On Thu, Aug 15, 2019 at 02:58:01PM +0200, Max Staudt wrote: >> ltc2990 will now use device_property_read_u32_array() instead of >> of_property_read_u32_array() - allowing the use of software nodes >> via fwnode_create_software_node(). >> >> This allows code using i2c_new_device() to specify a default >> measurement mode for the LTC2990. >> >> Signed-off-by: Max Staudt > > Applied to hwmon-next. Hi Guenter, Thank you for reviewing and taking this patch, as well as teaching me about swnode. Max
Re: [PATCH v3 2/3] hwmon/ltc2990: Generalise DT to fwnode support
On Thu, Aug 15, 2019 at 02:58:01PM +0200, Max Staudt wrote: > ltc2990 will now use device_property_read_u32_array() instead of > of_property_read_u32_array() - allowing the use of software nodes > via fwnode_create_software_node(). > > This allows code using i2c_new_device() to specify a default > measurement mode for the LTC2990. > > Signed-off-by: Max Staudt Applied to hwmon-next. Thanks, Guenter
[PATCH v3 1/3] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
This is the i2c-icy driver for the ICY board for Amiga computers.
It connects a PCF8584 I2C controller to the Zorro bus, providing I2C
connectivity. The original documentation can be found on Aminet:
https://aminet.net/package/docs/hard/icy
IRQ support is currently not implemented, as i2c-algo-pcf is built for
the ISA bus and a straight implementation of the same stack locks up a
Zorro machine.
v3: Fixed %pa format string
Dropped adapter class.
Clarified licence.
Removed clock parameter.
v2: Matched function names to callbacks from i2c-algo-pcf
Used z_readb()/z_writeb()
Removed BROKEN_ON_SMP in Kconfig
Moved LTC2990 to a separate commit
Signed-off-by: Max Staudt
---
MAINTAINERS | 6 ++
drivers/i2c/busses/Kconfig | 11 +++
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-icy.c | 183 +++
4 files changed, 201 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-icy.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 1be025959..70336c083 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7705,6 +7705,12 @@ S: Maintained
F: drivers/mfd/lpc_ich.c
F: drivers/gpio/gpio-ich.c
+ICY I2C DRIVER
+M: Max Staudt
+L: linux-...@vger.kernel.org
+S: Maintained
+F: drivers/i2c/busses/i2c-icy.c
+
IDE SUBSYSTEM
M: "David S. Miller"
L: linux-...@vger.kernel.org
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index ee5dfb5ae..9e57e1101 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1300,6 +1300,17 @@ config I2C_ELEKTOR
This support is also available as a module. If so, the module
will be called i2c-elektor.
+config I2C_ICY
+ tristate "ICY Zorro card"
+ depends on ZORRO
+ select I2C_ALGOPCF
+ help
+ This supports the PCF8584 Zorro bus I2C adapter, known as ICY.
+ Say Y if you own such an adapter.
+
+ This support is also available as a module. If so, the module
+ will be called i2c-icy.
+
config I2C_MLXCPLD
tristate "Mellanox I2C driver"
depends on X86_64
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index a3245231b..d0e1c3d4e 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -139,6 +139,7 @@ obj-$(CONFIG_I2C_BCM_KONA) += i2c-bcm-kona.o
obj-$(CONFIG_I2C_BRCMSTB) += i2c-brcmstb.o
obj-$(CONFIG_I2C_CROS_EC_TUNNEL) += i2c-cros-ec-tunnel.o
obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
+obj-$(CONFIG_I2C_ICY) += i2c-icy.o
obj-$(CONFIG_I2C_MLXCPLD) += i2c-mlxcpld.o
obj-$(CONFIG_I2C_OPAL) += i2c-opal.o
obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
diff --git a/drivers/i2c/busses/i2c-icy.c b/drivers/i2c/busses/i2c-icy.c
new file mode 100644
index 0..edac515da
--- /dev/null
+++ b/drivers/i2c/busses/i2c-icy.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * I2C driver for stand-alone PCF8584 style adapters on Zorro cards
+ *
+ * Original ICY documentation can be found on Aminet:
+ * https://aminet.net/package/docs/hard/icy
+ *
+ * There has been a modern community re-print of this design in 2019:
+ * https://www.a1k.org/forum/index.php?threads/70106/
+ *
+ * The card is basically a Philips PCF8584 connected straight to the
+ * beginning of the AutoConfig'd address space (register S1 on base+2),
+ * with /INT on /INT2 on the Zorro bus.
+ *
+ * Copyright (c) 2019 Max Staudt
+ *
+ * This started as a fork of i2c-elektor.c and has evolved since.
+ * Thanks go to its authors for providing a base to grow on.
+ *
+ *
+ * IRQ support is currently not implemented.
+ *
+ * As it turns out, i2c-algo-pcf is really written with i2c-elektor's
+ * edge-triggered ISA interrupts in mind, while the Amiga's Zorro bus has
+ * level-triggered interrupts. This means that once an interrupt occurs, we
+ * have to tell the PCF8584 to shut up immediately, or it will keep the
+ * interrupt line busy and cause an IRQ storm.
+
+ * However, because of the PCF8584's host-side protocol, there is no good
+ * way to just quieten it without side effects. Rather, we have to perform
+ * the next read/write operation straight away, which will reset the /INT
+ * pin. This entails re-designing the core of i2c-algo-pcf in the future.
+ * For now, we never request an IRQ from the PCF8584, and poll it instead.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+#include
+
+#include "../algos/i2c-algo-pcf.h"
+
+
+struct icy_i2c {
+ struct i2c_adapter adapter;
+
+ void __iomem *reg_s0;
+ void __iomem *reg_s1;
+};
+
+
+
+/*
+ * Functions called by i2c-algo-pcf
+ */
+static void icy_pcf_setpcf(void *data, int ctl, int val)
+{
+ struct icy_i2c *i2c = (struct icy_i2c *)data;
+
+ u8 __iomem *address = ctl ? i2c->reg_s1 : i2c->reg_s0;
+
+ z_writeb(val, address);
+}
+
+static int
[PATCH v3 3/3] i2c/busses/i2c-icy: Add LTC2990 present on 2019 board revision
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
---
drivers/i2c/busses/Kconfig | 3 +++
drivers/i2c/busses/i2c-icy.c | 56
2 files changed, 59 insertions(+)
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(>dev, sizeof(*i2c), GFP_KERNEL);
@@ -147,6 +171,35 @@ static int icy_probe(struct zorro_dev *z,
dev_info(>dev, "ICY I2C controller at %pa, IRQ not implemented\n",
>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(>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(>adapter,
+ _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(>dev);
+ i2c_unregister_device(i2c->ltc2990_client);
+ fwnode_remove_software_node(i2c->ltc2990_fwnode);
+
i2c_del_adapter(>adapter);
}
--
2.11.0
[PATCH v3 2/3] hwmon/ltc2990: Generalise DT to fwnode support
ltc2990 will now use device_property_read_u32_array() instead of
of_property_read_u32_array() - allowing the use of software nodes
via fwnode_create_software_node().
This allows code using i2c_new_device() to specify a default
measurement mode for the LTC2990.
Signed-off-by: Max Staudt
---
drivers/hwmon/ltc2990.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/ltc2990.c b/drivers/hwmon/ltc2990.c
index f9431ad43..e06619ac9 100644
--- a/drivers/hwmon/ltc2990.c
+++ b/drivers/hwmon/ltc2990.c
@@ -206,7 +206,6 @@ static int ltc2990_i2c_probe(struct i2c_client *i2c,
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))
@@ -218,9 +217,10 @@ static int ltc2990_i2c_probe(struct i2c_client *i2c,
data->i2c = i2c;
- if (of_node) {
- ret = of_property_read_u32_array(of_node, "lltc,meas-mode",
-data->mode, 2);
+ if (i2c->dev.of_node || i2c->dev.fwnode) {
+ ret = device_property_read_u32_array(>dev,
+"lltc,meas-mode",
+data->mode, 2);
if (ret < 0)
return ret;
--
2.11.0
Re: [PATCH v2 1/4] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
> The callback, just like getclock(), is from the existing i2c-algo-pcf, > which I don't want to touch right now. So I'm afraid it has to stay, > even if it returns a fixed number. Sure. I was just thinking loud and did not expect you to do it. Maybe I will do it once your driver is upstream. > Touching or forking i2c-algo-pcf will be necessary in order to > implement IRQ support, if I ever get around to it. I've wasted too > much time on debugging IRQ support, and would rather do it another > time, if ever ;) I've left a comment block at the start of i2c-icy.c > for anyone interested in the gory details. Yeah, I read it. I like such documentation. Really helpful! > If this is okay with you, I'll send another round of patches. :) Perfect! signature.asc Description: PGP signature
Re: [PATCH v2 1/4] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
On 08/15/2019 02:04 PM, Wolfram Sang wrote: >> When the PCF8584 is addressed as slave, this register >> must be loaded with the 7-bit I 2 C-bus address to which the >> PCF8584 is to respond. During initialization, the own >> address register S0' must be written to, regardless >> whether it is later used. > > I see. It must be written a non-zero value to leave the monitor mode. > But this really needs no callback, we can hardcode any non-zero value. > If slave support is (ever) to be implemented, the own address will come > from the I2C core. The callback, just like getclock(), is from the existing i2c-algo-pcf, which I don't want to touch right now. So I'm afraid it has to stay, even if it returns a fixed number. Touching or forking i2c-algo-pcf will be necessary in order to implement IRQ support, if I ever get around to it. I've wasted too much time on debugging IRQ support, and would rather do it another time, if ever ;) I've left a comment block at the start of i2c-icy.c for anyone interested in the gory details. If this is okay with you, I'll send another round of patches. :) Thanks! Max
Re: [PATCH v2 1/4] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
> My suggestion is to not touch i2c-elektor at all for now, and remove > the 'clock' parameter from the first iteration of i2c-icy. It can be > added back in case someone complains, which I deem unlikely. Full ack. > When the PCF8584 is addressed as slave, this register > must be loaded with the 7-bit I 2 C-bus address to which the > PCF8584 is to respond. During initialization, the own > address register S0' must be written to, regardless > whether it is later used. I see. It must be written a non-zero value to leave the monitor mode. But this really needs no callback, we can hardcode any non-zero value. If slave support is (ever) to be implemented, the own address will come from the I2C core. > Okay, so we don't care. Cool, then it's safe to kick the 'clock' > parameter from i2c-icy. All 2019 boards (which should be the vast > majority in existence) came with a 12 MHz oscillator AFAIK, so the > default should be good. Most drivers don't allow users to change the bus speed, so I think it is safe to remove. signature.asc Description: PGP signature
Re: [PATCH v2 1/4] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
On 08/15/2019 01:48 PM, Wolfram Sang wrote: >> Well, the other option is to remove it, and then add it back once >> somebody complains - which is unlikely to happen. The clock parameter >> is PCF8584 specific anyway, and I think removing it is a good option, > > My suggestion is to do that incrementally. First, get your driver > accepted. Second, do the cleanups which affect elektor as well later. My suggestion is to not touch i2c-elektor at all for now, and remove the 'clock' parameter from the first iteration of i2c-icy. It can be added back in case someone complains, which I deem unlikely. >> as I've done the same with getown() (where in i2c-elektor, 'own' sets >> the PCF8584's own address). > > I wondered about that. Can the PCF8584 really act as a slave, too? > Somewhen I need to check its datasheet. Yes it can, but this driver does not implement this. In fact, I just remembered that the own address, while set, is not used at all in master mode! Quote from the datasheet: https://www.nxp.com/docs/en/data-sheet/PCF8584.pdf 6.4 Own address register S0' When the PCF8584 is addressed as slave, this register must be loaded with the 7-bit I 2 C-bus address to which the PCF8584 is to respond. During initialization, the own address register S0' must be written to, regardless whether it is later used. >> Question is, if I remove the parameter, I'd like it to be >> non-destructive. Do you know of anything that can go wrong if the I2C >> master is running the bus on a wrong clock? > > Not sure if I understand you correctly, but if the bus freq is too fast > then devices won't respond. Too slow is not a problem. Okay, so we don't care. Cool, then it's safe to kick the 'clock' parameter from i2c-icy. All 2019 boards (which should be the vast majority in existence) came with a 12 MHz oscillator AFAIK, so the default should be good. Max
Re: [PATCH v2 1/4] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
> Well, the other option is to remove it, and then add it back once > somebody complains - which is unlikely to happen. The clock parameter > is PCF8584 specific anyway, and I think removing it is a good option, My suggestion is to do that incrementally. First, get your driver accepted. Second, do the cleanups which affect elektor as well later. > as I've done the same with getown() (where in i2c-elektor, 'own' sets > the PCF8584's own address). I wondered about that. Can the PCF8584 really act as a slave, too? Somewhen I need to check its datasheet. > Question is, if I remove the parameter, I'd like it to be > non-destructive. Do you know of anything that can go wrong if the I2C > master is running the bus on a wrong clock? Not sure if I understand you correctly, but if the bus freq is too fast then devices won't respond. Too slow is not a problem. signature.asc Description: PGP signature
Re: [PATCH v2 1/4] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
On 08/15/2019 09:12 AM, Wolfram Sang wrote: >> I kept it in because i2c-elektor also exposes it in the same way, and >> it seems useful. > > Yeah, I noticed. I don't know how useful it is in practice (same for the > getown callback) but I don't have better ideas, so let's just keep this > to be consistent. Well, the other option is to remove it, and then add it back once somebody complains - which is unlikely to happen. The clock parameter is PCF8584 specific anyway, and I think removing it is a good option, as I've done the same with getown() (where in i2c-elektor, 'own' sets the PCF8584's own address). Question is, if I remove the parameter, I'd like it to be non-destructive. Do you know of anything that can go wrong if the I2C master is running the bus on a wrong clock? Thanks Max
Re: [PATCH v2 1/4] i2c/busses: Add i2c-icy for I2C on m68k/Amiga
> I kept it in because i2c-elektor also exposes it in the same way, and > it seems useful. Yeah, I noticed. I don't know how useful it is in practice (same for the getown callback) but I don't have better ideas, so let's just keep this to be consistent. > > Your SPDX header says GPL 2.0 only. > > Thanks! I'll change it to "GPL v2", but unfortunately there is no > option for "only" in include/linux/module.h. That's the correct option. Thanks! signature.asc Description: PGP signature
