Skip to contentSkip to conversation

i2c: muxes: pca954x: look up width from chip_desc

PATCH New Chris Packham 2 participants 4 messages Sep 28, 2017 – Sep 29, 2017 I2C muxes

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 i2c: muxes: pca954x: look up width from chip_desc

Commit 8e6eda7cda6c ("drivers/i2c/muxes/pca954x: Add pca9547 I2C mux support") introduced a chip_desc for the pca954x devices but failed to update pca954x_ofdata_to_platdata() to be aware of it. Make pca954x_ofdata_to_platdata() lookup the chip_desc to validate the device width.

---

 drivers/i2c/muxes/pca954x.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/i2c/muxes/pca954x.c +6 -1
index 01ca1ff48db9..2b70ff82bdd0 100644
@@ -27,6 +27,7 @@ struct chip_desc {
 		pca954x_ismux = 0,
 		pca954x_isswi,
 	} muxtype;
+	u32 width;
 };
 
 struct pca954x_priv {
@@ -39,14 +40,17 @@ static const struct chip_desc chips[] = {
 	[PCA9544] = {
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
+		.width = 4,
 	},
 	[PCA9547] = {
 		.enable = 0x8,
 		.muxtype = pca954x_ismux,
+		.width = 8,
 	},
 	[PCA9548] = {
 		.enable = 0x8,
 		.muxtype = pca954x_isswi,
+		.width = 8,
 	},
 };
 
@@ -89,13 +93,14 @@ static const struct udevice_id pca954x_ids[] = {
 static int pca954x_ofdata_to_platdata(struct udevice *dev)
 {
 	struct pca954x_priv *priv = dev_get_priv(dev);
+	const struct chip_desc *chip = &chips[dev_get_driver_data(dev)];
 
 	priv->addr = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "reg", 0);
 	if (!priv->addr) {
 		debug("MUX not found\n");
 		return -ENODEV;
 	}
-	priv->width = dev_get_driver_data(dev);
+	priv->width = chip->width;
 
 	if (!priv->width) {
 		debug("No I2C MUX width specified\n");
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/1 i2c: muxes: pca954x: look up width from chip_desc

Quoted 58 lines · On Fri, 29 Sep 2017 10:53:36 +1300 Chris Packham <[email protected]>…

Commit 8e6eda7cda6c ("drivers/i2c/muxes/pca954x: Add pca9547 I2C mux support") introduced a chip_desc for the pca954x devices but failed to update pca954x_ofdata_to_platdata() to be aware of it. Make pca954x_ofdata_to_platdata() lookup the chip_desc to validate the device width.

---

 drivers/i2c/muxes/pca954x.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/i2c/muxes/pca954x.c +6 -1
index 01ca1ff48db9..2b70ff82bdd0 100644
@@ -27,6 +27,7 @@ struct chip_desc {
 		pca954x_ismux = 0,
 		pca954x_isswi,
 	} muxtype;
+	u32 width;
 };
 
 struct pca954x_priv {
@@ -39,14 +40,17 @@ static const struct chip_desc chips[] = {
 	[PCA9544] = {
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
+		.width = 4,
 	},
 	[PCA9547] = {
 		.enable = 0x8,
 		.muxtype = pca954x_ismux,
+		.width = 8,
 	},
 	[PCA9548] = {
 		.enable = 0x8,
 		.muxtype = pca954x_isswi,
+		.width = 8,
 	},
 };
 
@@ -89,13 +93,14 @@ static const struct udevice_id pca954x_ids[] = {
 static int pca954x_ofdata_to_platdata(struct udevice *dev)
 {
 	struct pca954x_priv *priv = dev_get_priv(dev);
+	const struct chip_desc *chip =
 &chips[dev_get_driver_data(dev)]; 
 	priv->addr = fdtdec_get_int(gd->fdt_blob,
 dev_of_offset(dev), "reg", 0); if (!priv->addr) {
 		debug("MUX not found\n");
 		return -ENODEV;
 	}
-	priv->width = dev_get_driver_data(dev);
+	priv->width = chip->width;
 
 	if (!priv->width) {
 		debug("No I2C MUX width specified\n");
#2 lore raw

PATCH 1/1 i2c: muxes: pca954x: look up width from chip_desc

In reply to Chris Packham
Quoted 8 lines · On Fri, 29 Sep 2017 10:53:36 +1300 Chris Packham <[email protected]>…

struct pca954x_priv {

drivers/i2c/muxes/pca954x.c +1 -0
@@ -39,14 +40,17 @@ static const struct chip_desc chips[] = {
 	[PCA9544] = {
 		.enable = 0x4,
 		.muxtype = pca954x_ismux,
+		.width = 4,
 	},
 	[PCA9547] = {

drivers/i2c/muxes/pca954x.c static const struct chip_desc chips[] = { · line 55

Quoted 10 lines
drivers/i2c/muxes/pca954x.c +2 -0
 		.enable = 0x8,
 		.muxtype = pca954x_ismux,
+		.width = 8,
 	},
 	[PCA9548] = {
 		.enable = 0x8,
 		.muxtype = pca954x_isswi,
+		.width = 8,
 	},
 };

Hmm, looking at this now, isn't one of the enable or width fields redundant? They both have same values in all entries.

Marek

#3 lore raw

PATCH 1/1 i2c: muxes: pca954x: look up width from chip_desc

On 30/09/2017 2:43 AM, "Marek Behún" <[email protected]> wrote:

Quoted 8 lines · On Fri, 29 Sep 2017 10:53:36 +1300 Chris Packham <[email protected]>…

struct pca954x_priv {

drivers/i2c/muxes/pca954x.c +1 -0
@@ -39,14 +40,17 @@ static const struct chip_desc chips[] = {
      [PCA9544] = {
              .enable = 0x4,
              .muxtype = pca954x_ismux,
+             .width = 4,
      },
      [PCA9547] = {

drivers/i2c/muxes/pca954x.c static const struct chip_desc chips[] = { · line 55

Quoted 10 lines
drivers/i2c/muxes/pca954x.c +2 -0
              .enable = 0x8,
              .muxtype = pca954x_ismux,
+             .width = 8,
      },
      [PCA9548] = {
              .enable = 0x8,
              .muxtype = pca954x_isswi,
+             .width = 8,
      },
 };

Hmm, looking at this now, isn't one of the enable or width fields redundant? They both have same values in all entries.

They happen to have the same value but they have different meanings. In fact enable isn't actually used for the isswi code path.