Re: [U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
Hi Heiko, Hello Simon, Lukasz, Am 30.03.2014 01:17, schrieb Simon Glass: Hi Lukasz, On 27 March 2014 11:33, Lukasz Majewskil.majew...@samsung.com wrote: Hi Simon, Heiko From: Aaron Durbinadur...@chromium.org The current pmic i2c code assumes the current i2c bus is the same as the pmic device's bus. There is nothing ensuring that to be true. Therefore, select the proper bus before performing a transaction. Signed-off-by: Aaron Durbinadur...@chromium.org Signed-off-by: Simon Glasss...@chromium.org Reviewed-by: Simon Glasss...@chromium.org --- drivers/power/power_i2c.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index ac76870..594cd11 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p-bus); + Hadn't we had a discussion about this explicit setting of I2C some time ago? I thought that this problem was solved within the I2C rework. Also I might be wrong, so please correct me if I'm wrong. Isn't the I2C_SET_BUS() macro regarded as a obsolete after the I2C rework? Agreed that would be ideal, but we would have to pass the bus number of the i2c_read/write() functions. I don't believe the i2c code has got that far yet. Yes, thats the plan, but first, all i2c driver must be converted to the new framework. After that we could start with such an approach (or device model is ready and we can switch to it ...) I know that there is a time line for introducing device model, but is there any for switching I2C to the new approach? I think about deleting obsolete/unmaintained boards, which will not switch to new I2C approach. Unfortunately it doesn't work without this patch. Yes ... If we have all i2c driver running with the new framework, we can get rid of I2C_SET_BUS defines, and simply use i2c_set_bus_num() which is a simple cleanup patch. Ok, I see. bye, Heiko -- Best regards, Lukasz Majewski Samsung RD Institute Poland (SRPOL) | Linux Platform Group ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
Hi Simon, Hi Lukasz, On 27 March 2014 11:33, Lukasz Majewski l.majew...@samsung.com wrote: Hi Simon, Heiko From: Aaron Durbin adur...@chromium.org The current pmic i2c code assumes the current i2c bus is the same as the pmic device's bus. There is nothing ensuring that to be true. Therefore, select the proper bus before performing a transaction. Signed-off-by: Aaron Durbin adur...@chromium.org Signed-off-by: Simon Glass s...@chromium.org Reviewed-by: Simon Glass s...@chromium.org --- drivers/power/power_i2c.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index ac76870..594cd11 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p-bus); + Hadn't we had a discussion about this explicit setting of I2C some time ago? I thought that this problem was solved within the I2C rework. Also I might be wrong, so please correct me if I'm wrong. Isn't the I2C_SET_BUS() macro regarded as a obsolete after the I2C rework? Agreed that would be ideal, but we would have to pass the bus number of the i2c_read/write() functions. I don't believe the i2c code has got that far yet. Unfortunately it doesn't work without this patch. If Heiko doesn't object, then I won't protest. Regards, Simon -- Best regards, Lukasz Majewski Samsung RD Institute Poland (SRPOL) | Linux Platform Group ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
Hello Lukasz, Am 31.03.2014 08:17, schrieb Lukasz Majewski: Hi Heiko, Hello Simon, Lukasz, Am 30.03.2014 01:17, schrieb Simon Glass: Hi Lukasz, On 27 March 2014 11:33, Lukasz Majewskil.majew...@samsung.com wrote: Hi Simon, Heiko From: Aaron Durbinadur...@chromium.org The current pmic i2c code assumes the current i2c bus is the same as the pmic device's bus. There is nothing ensuring that to be true. Therefore, select the proper bus before performing a transaction. Signed-off-by: Aaron Durbinadur...@chromium.org Signed-off-by: Simon Glasss...@chromium.org Reviewed-by: Simon Glasss...@chromium.org --- drivers/power/power_i2c.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index ac76870..594cd11 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p-bus); + Hadn't we had a discussion about this explicit setting of I2C some time ago? I thought that this problem was solved within the I2C rework. Also I might be wrong, so please correct me if I'm wrong. Isn't the I2C_SET_BUS() macro regarded as a obsolete after the I2C rework? Agreed that would be ideal, but we would have to pass the bus number of the i2c_read/write() functions. I don't believe the i2c code has got that far yet. Yes, thats the plan, but first, all i2c driver must be converted to the new framework. After that we could start with such an approach (or device model is ready and we can switch to it ...) I know that there is a time line for introducing device model, but is there any for switching I2C to the new approach? I am not aware of a plan ... I think about deleting obsolete/unmaintained boards, which will not switch to new I2C approach. Hmm... this would be a long list, as there are the following driver which need a conversion: obj-$(CONFIG_BFIN_TWI_I2C) += bfin-twi_i2c.o obj-$(CONFIG_DRIVER_DAVINCI_I2C) += davinci_i2c.o obj-$(CONFIG_DW_I2C) += designware_i2c.o obj-$(CONFIG_I2C_MVTWSI) += mvtwsi.o obj-$(CONFIG_I2C_MV) += mv_i2c.o obj-$(CONFIG_I2C_MXS) += mxs_i2c.o obj-$(CONFIG_PCA9564_I2C) += pca9564_i2c.o obj-$(CONFIG_TSI108_I2C) += tsi108_i2c.o obj-$(CONFIG_U8500_I2C) += u8500_i2c.o obj-$(CONFIG_SH_SH7734_I2C) += sh_sh7734_i2c.o Also some drivers in cpu dirs ... grep for HARD_I2C in u-boot source (one Goal is to get rid of HARD_I2C). ./arch/powerpc/cpu/mpc8xx/i2c.c ./arch/powerpc/cpu/mpc8260/commproc.c ./arch/powerpc/cpu/mpc8260/i2c.c ./arch/powerpc/cpu/mpc5xxx/i2c.c ./arch/powerpc/cpu/mpc824x/drivers/i2c/i2c.c ./arch/powerpc/cpu/mpc512x/i2c.c [...] bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
Hello Lukasz, Am 31.03.2014 16:36, schrieb Lukasz Majewski: Hi Simon, Hi Lukasz, On 27 March 2014 11:33, Lukasz Majewskil.majew...@samsung.com wrote: Hi Simon, Heiko From: Aaron Durbinadur...@chromium.org The current pmic i2c code assumes the current i2c bus is the same as the pmic device's bus. There is nothing ensuring that to be true. Therefore, select the proper bus before performing a transaction. Signed-off-by: Aaron Durbinadur...@chromium.org Signed-off-by: Simon Glasss...@chromium.org Reviewed-by: Simon Glasss...@chromium.org --- drivers/power/power_i2c.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index ac76870..594cd11 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p-bus); + Hadn't we had a discussion about this explicit setting of I2C some time ago? I thought that this problem was solved within the I2C rework. Also I might be wrong, so please correct me if I'm wrong. Isn't the I2C_SET_BUS() macro regarded as a obsolete after the I2C rework? Agreed that would be ideal, but we would have to pass the bus number of the i2c_read/write() functions. I don't believe the i2c code has got that far yet. Unfortunately it doesn't work without this patch. If Heiko doesn't object, then I won't protest. It s okay for me, so to clarify: Acked-by: Heiko Schocher h...@denx.de bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
Hello Simon, Lukasz, Am 30.03.2014 01:17, schrieb Simon Glass: Hi Lukasz, On 27 March 2014 11:33, Lukasz Majewskil.majew...@samsung.com wrote: Hi Simon, Heiko From: Aaron Durbinadur...@chromium.org The current pmic i2c code assumes the current i2c bus is the same as the pmic device's bus. There is nothing ensuring that to be true. Therefore, select the proper bus before performing a transaction. Signed-off-by: Aaron Durbinadur...@chromium.org Signed-off-by: Simon Glasss...@chromium.org Reviewed-by: Simon Glasss...@chromium.org --- drivers/power/power_i2c.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index ac76870..594cd11 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p-bus); + Hadn't we had a discussion about this explicit setting of I2C some time ago? I thought that this problem was solved within the I2C rework. Also I might be wrong, so please correct me if I'm wrong. Isn't the I2C_SET_BUS() macro regarded as a obsolete after the I2C rework? Agreed that would be ideal, but we would have to pass the bus number of the i2c_read/write() functions. I don't believe the i2c code has got that far yet. Yes, thats the plan, but first, all i2c driver must be converted to the new framework. After that we could start with such an approach (or device model is ready and we can switch to it ...) Unfortunately it doesn't work without this patch. Yes ... If we have all i2c driver running with the new framework, we can get rid of I2C_SET_BUS defines, and simply use i2c_set_bus_num() which is a simple cleanup patch. bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
Hi Lukasz, On 27 March 2014 11:33, Lukasz Majewski l.majew...@samsung.com wrote: Hi Simon, Heiko From: Aaron Durbin adur...@chromium.org The current pmic i2c code assumes the current i2c bus is the same as the pmic device's bus. There is nothing ensuring that to be true. Therefore, select the proper bus before performing a transaction. Signed-off-by: Aaron Durbin adur...@chromium.org Signed-off-by: Simon Glass s...@chromium.org Reviewed-by: Simon Glass s...@chromium.org --- drivers/power/power_i2c.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c index ac76870..594cd11 100644 --- a/drivers/power/power_i2c.c +++ b/drivers/power/power_i2c.c @@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val) if (check_reg(p, reg)) return -1; + I2C_SET_BUS(p-bus); + Hadn't we had a discussion about this explicit setting of I2C some time ago? I thought that this problem was solved within the I2C rework. Also I might be wrong, so please correct me if I'm wrong. Isn't the I2C_SET_BUS() macro regarded as a obsolete after the I2C rework? Agreed that would be ideal, but we would have to pass the bus number of the i2c_read/write() functions. I don't believe the i2c code has got that far yet. Unfortunately it doesn't work without this patch. Regards, Simon ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
Hi Simon, Heiko
From: Aaron Durbin adur...@chromium.org
The current pmic i2c code assumes the current i2c bus is
the same as the pmic device's bus. There is nothing ensuring
that to be true. Therefore, select the proper bus before performing
a transaction.
Signed-off-by: Aaron Durbin adur...@chromium.org
Signed-off-by: Simon Glass s...@chromium.org
Reviewed-by: Simon Glass s...@chromium.org
---
drivers/power/power_i2c.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c
index ac76870..594cd11 100644
--- a/drivers/power/power_i2c.c
+++ b/drivers/power/power_i2c.c
@@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val)
if (check_reg(p, reg))
return -1;
+ I2C_SET_BUS(p-bus);
+
Hadn't we had a discussion about this explicit setting of I2C some time
ago? I thought that this problem was solved within the I2C rework.
Also I might be wrong, so please correct me if I'm wrong. Isn't the
I2C_SET_BUS() macro regarded as a obsolete after the I2C rework?
switch (pmic_i2c_tx_num) {
case 3:
if (p-sensor_byte_order ==
PMIC_SENSOR_BYTE_ORDER_BIG) { @@ -66,6 +68,8 @@ int
pmic_reg_read(struct pmic *p, u32 reg, u32 *val) if (check_reg(p,
reg)) return -1;
+ I2C_SET_BUS(p-bus);
+
if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num))
return -1;
--
Best regards,
Lukasz Majewski
Samsung RD Institute Poland (SRPOL) | Linux Platform Group
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 04/10] power: Explicitly select pmic device's bus
From: Aaron Durbin adur...@chromium.org
The current pmic i2c code assumes the current i2c bus is
the same as the pmic device's bus. There is nothing ensuring
that to be true. Therefore, select the proper bus before performing
a transaction.
Signed-off-by: Aaron Durbin adur...@chromium.org
Signed-off-by: Simon Glass s...@chromium.org
Reviewed-by: Simon Glass s...@chromium.org
---
drivers/power/power_i2c.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/power/power_i2c.c b/drivers/power/power_i2c.c
index ac76870..594cd11 100644
--- a/drivers/power/power_i2c.c
+++ b/drivers/power/power_i2c.c
@@ -23,6 +23,8 @@ int pmic_reg_write(struct pmic *p, u32 reg, u32 val)
if (check_reg(p, reg))
return -1;
+ I2C_SET_BUS(p-bus);
+
switch (pmic_i2c_tx_num) {
case 3:
if (p-sensor_byte_order == PMIC_SENSOR_BYTE_ORDER_BIG) {
@@ -66,6 +68,8 @@ int pmic_reg_read(struct pmic *p, u32 reg, u32 *val)
if (check_reg(p, reg))
return -1;
+ I2C_SET_BUS(p-bus);
+
if (i2c_read(pmic_i2c_addr, reg, 1, buf, pmic_i2c_tx_num))
return -1;
--
1.9.1.423.g4596e3a
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
