I2C driver updates

For I2C clockactivity field is added for OMAP3 and OMAP4 that defines how the interface (OCP) and functional (system) clocks behave when the I2C module is idle.

The configuration of the clock activity bit field (per TRM) is as follows:
0x0: Both clocks can be cut off
0x1: Only OCP clock must be kept active; system clock
can be cut off
0x3: Both clocks must be kept active
0x2: Only system clock must be kept active; OCP clock
can be cut off

The patch makes 0x2(CLOCKACT_TEST_ICLK) the default for OMAP3 and OMAP4.

Signed-off-by Shubhrajyoti D <[email protected]>

Signed-off-by Jon Hunter <[email protected]>

Add a function omap_device_reset() to reset all the hwmods in the hwmod device. This is intended to be used by device drivers to reset all the hwmods in the device. This is needed to support some ips like i2c which may have to do reset in error paths or in errata.

Signed-off-by Shubhrajyoti D <[email protected]>

Under some error conditions the i2c driver may do a reset. Adding a reset field and support in the device-specific code to aid error-recovery.

Signed-off-by Shubhrajyoti D <[email protected]>

Under some error conditions the i2c driver may do a reset. Adding a reset field and support in the device-specific code to aid error-recovery.

Signed-off-by Shubhrajyoti D<[email protected]>

---
  arch/arm/plat-omap/i2c.c |    2 ++
  include/linux/i2c-omap.h |    1 +
  2 files changed, 3 insertions(+), 0 deletions(-)

arch/arm/plat-omap/i2c.c +0 -0

index db071bc..6cddde2 100644

Hi Shubhro,

Quoted 10 lines · Shubhrajyoti D

Under some error conditions the i2c driver may do a reset. Adding a reset field and support in the device-specific code to aid error-recovery.

Signed-off-by Shubhrajyoti D<[email protected]>

---
  arch/arm/plat-omap/i2c.c |    2 ++
  include/linux/i2c-omap.h |    1 +
  2 files changed, 3 insertions(+), 0 deletions(-)

arch/arm/plat-omap/i2c.c static inline int omap2_i2c_add_bus(int bus_id) · line 184

Quoted 11 lines

arch/arm/plat-omap/i2c.c +2 -0

index db071bc..6cddde2 100644
@@ -179,6 +179,8 @@ static inline int omap2_i2c_add_bus(int bus_id)
       */
      if (cpu_is_omap34xx())
          pdata->set_mpu_wkup_lat =
 omap_pm_set_max_mpu_wakeup_lat_compat;
+
+    pdata->device_reset = omap_device_reset;

We should avoid introducing any new pdata function pointers since we are in the process of removing them for DT support.

Regards,
Benoit

Hi Benoit,

Quoted 13 lines · Benoit

Hi Shubhro,

Quoted 10 lines · Shubhrajyoti D

Under some error conditions the i2c driver may do a reset. Adding a reset field and support in the device-specific code to aid error-recovery.

Signed-off-by Shubhrajyoti D<[email protected]>

---
  arch/arm/plat-omap/i2c.c |    2 ++
  include/linux/i2c-omap.h |    1 +
  2 files changed, 3 insertions(+), 0 deletions(-)

Quoted 14 lines

arch/arm/plat-omap/i2c.c static inline int omap2_i2c_add_bus(int bus_id) · line 184

Quoted 11 lines

arch/arm/plat-omap/i2c.c +2 -0

index db071bc..6cddde2 100644
@@ -179,6 +179,8 @@ static inline int omap2_i2c_add_bus(int bus_id)
       */
      if (cpu_is_omap34xx())
          pdata->set_mpu_wkup_lat =
 omap_pm_set_max_mpu_wakeup_lat_compat;
+
+    pdata->device_reset = omap_device_reset;

We should avoid introducing any new pdata function pointers since we are in the process of removing them for DT support.

However if the driver has to do a reset due to some error what is the recommended way?
Currently we used to access the SYSC directly I am removing the same and introducing this function pointer.

(quoted diff) +0 -6

-  The reset in the driver at init is not needed anymore as the
   hwmod framework takes care of reseting it.
-  Reset is removed from omap_i2c_init, which was called
   not only during probe, but also after time out and error handling.
   device_reset were added in those places to effect the reset.
-  Earlier the hwmod SYSC settings were over-written in the driver.
   Removing the same and letting the hwmod take care of the settings.
-  Clean up the SYSS_RESETDONE_MASK macro as it is no longer needed.
-  Clean up the SYSCONFIG SYSC bit definition macros.
-  Fix the typos in wakeup.

Signed-off-by Shubhrajyoti D <[email protected]>

---
 drivers/i2c/busses/i2c-omap.c |   83 +++++++++++-----------------------------
 1 files changed, 23 insertions(+), 60 deletions(-)

Hi

Quoted 11 lines · Shubhrajyoti D

(quoted diff) +0 -6

-  The reset in the driver at init is not needed anymore as the
   hwmod framework takes care of reseting it.
-  Reset is removed from omap_i2c_init, which was called
   not only during probe, but also after time out and error handling.
   device_reset were added in those places to effect the reset.
-  Earlier the hwmod SYSC settings were over-written in the driver.
   Removing the same and letting the hwmod take care of the settings.
-  Clean up the SYSS_RESETDONE_MASK macro as it is no longer needed.
-  Clean up the SYSCONFIG SYSC bit definition macros.
-  Fix the typos in wakeup.

Quoted 4 lines

Signed-off-by Shubhrajyoti D <[email protected]>

---
 drivers/i2c/busses/i2c-omap.c |   83 +++++++++++-----------------------------
 1 files changed, 23 insertions(+), 60 deletions(-)

This patch either needs to be acked by Ben with a note that it's okay for us to merge through the OMAP tree, or needs to be merged by Ben during the 3.4 merge window, after patches 1-3 have reached the mainline tree.

- Paul

Quoted 23 lines · Shubhrajyoti

Ben,

Quoted 19 lines · Paul Walmsley

Hi

Quoted 15 lines · Shubhrajyoti D

- ?The reset in the driver at init is not needed anymore as the ? ?hwmod framework takes care of reseting it.
- ?Reset is removed from omap_i2c_init, which was called
? ?not only during probe, but also after time out and error handling. ? ?device_reset were added in those places to effect the reset.
- ?Earlier the hwmod SYSC settings were over-written in the driver. ? ?Removing the same and letting the hwmod take care of the settings.
- ?Clean up the SYSS_RESETDONE_MASK macro as it is no longer needed.
- ?Clean up the SYSCONFIG SYSC bit definition macros.
- ?Fix the typos in wakeup.

Signed-off-by Shubhrajyoti D <[email protected]>

---
?drivers/i2c/busses/i2c-omap.c | ? 83 +++++++++++----------------------------- ?1 files changed, 23 insertions(+), 60 deletions(-)

Quoted 5 lines

Quoted 3 lines

This patch either needs to be acked by Ben with a note that it's okay for us to merge through the OMAP tree, or needs to be merged by Ben during the 3.4 merge window, after patches 1-3 have reached the mainline tree.

I agree.
Ben do you have any comments .

Quoted 5 lines

Quoted 4 lines

- Paul

Quoted 27 lines · Shubhrajyoti

Quoted 23 lines · Shubhrajyoti

Ben,

Quoted 19 lines · Paul Walmsley

Hi

Quoted 15 lines · Shubhrajyoti D

- ?The reset in the driver at init is not needed anymore as the ? ?hwmod framework takes care of reseting it.
- ?Reset is removed from omap_i2c_init, which was called
? ?not only during probe, but also after time out and error handling. ? ?device_reset were added in those places to effect the reset.
- ?Earlier the hwmod SYSC settings were over-written in the driver. ? ?Removing the same and letting the hwmod take care of the settings.
- ?Clean up the SYSS_RESETDONE_MASK macro as it is no longer needed.
- ?Clean up the SYSCONFIG SYSC bit definition macros.
- ?Fix the typos in wakeup.

Signed-off-by Shubhrajyoti D <[email protected]>

---
?drivers/i2c/busses/i2c-omap.c | ? 83 +++++++++++----------------------------- ?1 files changed, 23 insertions(+), 60 deletions(-)

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

This patch either needs to be acked by Ben with a note that it's okay for us to merge through the OMAP tree, or needs to be merged by Ben during the 3.4 merge window, after patches 1-3 have reached the mainline tree.

I agree.
Ben do you have any comments .

If there are no further comments can this be merged also ?

Quoted 33 lines · Shubhrajyoti

Quoted 31 lines · Shubhrajyoti

Quoted 27 lines · Shubhrajyoti

Ben,

Quoted 15 lines · Paul Walmsley

Hi

Quoted 11 lines · Shubhrajyoti D

(quoted diff) +0 -6

-  The reset in the driver at init is not needed anymore as the
   hwmod framework takes care of reseting it.
-  Reset is removed from omap_i2c_init, which was called
   not only during probe, but also after time out and error handling.
   device_reset were added in those places to effect the reset.
-  Earlier the hwmod SYSC settings were over-written in the driver.
   Removing the same and letting the hwmod take care of the settings.
-  Clean up the SYSS_RESETDONE_MASK macro as it is no longer needed.
-  Clean up the SYSCONFIG SYSC bit definition macros.
-  Fix the typos in wakeup.

Quoted 7 lines

Quoted 4 lines

Signed-off-by Shubhrajyoti D <[email protected]>

---
 drivers/i2c/busses/i2c-omap.c |   83 +++++++++++-----------------------------
 1 files changed, 23 insertions(+), 60 deletions(-)

This patch either needs to be acked by Ben with a note that it's okay for us to merge through the OMAP tree, or needs to be merged by Ben during the 3.4 merge window, after patches 1-3 have reached the mainline tree.

I agree.
Ben do you have any comments .

Quoted 1 line

If there are no further comments can this be merged also ?

As Benoit mentioned earlier, the addition of new pdata fields will cause problems with the DT support already queued for v3.3.

This series should be reworked on top of the DT support which Ben has now queued for v3.3 (my branch: for_3.3/i2c/misc)

Kevin

Quoted 14 lines · Kevin Hilman

Quoted 12 lines · Shubhrajyoti

Quoted 10 lines · Shubhrajyoti

Quoted 7 lines · Shubhrajyoti

Quoted 3 lines · Paul Walmsley

This patch either needs to be acked by Ben with a note that it's okay for us to merge through the OMAP tree, or needs to be merged by Ben during the 3.4 merge window, after patches 1-3 have reached the mainline tree.

I agree.
Ben do you have any comments .

Quoted 6 lines

Quoted 1 line

If there are no further comments can this be merged also ?

As Benoit mentioned earlier, the addition of new pdata fields will cause problems with the DT support already queued for v3.3.

This series should be reworked on top of the DT support which Ben has now queued for v3.3 (my branch: for_3.3/i2c/misc)

Yes will rework it.

Quoted 13 lines · Shubhrajyoti

Quoted 12 lines · Kevin Hilman

Quoted 10 lines · Shubhrajyoti

Quoted 8 lines · Shubhrajyoti

Quoted 7 lines · Shubhrajyoti

Quoted 3 lines · Paul Walmsley

This patch either needs to be acked by Ben with a note that it's okay for us to merge through the OMAP tree, or needs to be merged by Ben during the 3.4 merge window, after patches 1-3 have reached the mainline tree.

I agree.
Ben do you have any comments .

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

If there are no further comments can this be merged also ?

As Benoit mentioned earlier, the addition of new pdata fields will cause problems with the DT support already queued for v3.3.

This series should be reworked on top of the DT support which Ben has now queued for v3.3 (my branch: for_3.3/i2c/misc)

Yes will rework it.

Depending on what you plan to do, you'll probably need to coordinate this with Tony also. He's already pulled some of the patches from this series into his i2c branch.

The real question is how you plan to handle the device reset function, given that the driver should compile on a non-OMAP build (meaning that you can't call omap_device*() functions from the driver directly), nor should the driver touch the SYSCONFIG register directly.

- Paul

Quoted 4 lines · Paul Walmsley

The real question is how you plan to handle the device reset function, given that the driver should compile on a non-OMAP build (meaning that you can't call omap_device*() functions from the driver directly), nor should the driver touch the SYSCONFIG register directly.

Paul I thought through it myself not very clear.
- One way is to find some way for the dt to pass function pointer. Or maybe a flag that activates a static function / hwmod func.

Will give this a little more thought and get back. I have not any answer right now.

From an OMAP perspective, we could simply set that to point to

Quoted 15 lines · Shubhrajyoti

Quoted 14 lines · Kevin Hilman

Quoted 12 lines · Shubhrajyoti

Quoted 10 lines · Shubhrajyoti

Quoted 7 lines · Shubhrajyoti

Quoted 3 lines · Paul Walmsley

This patch either needs to be acked by Ben with a note that it's okay for us to merge through the OMAP tree, or needs to be merged by Ben during the 3.4 merge window, after patches 1-3 have reached the mainline tree.

I agree.
Ben do you have any comments .

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

If there are no further comments can this be merged also ?

As Benoit mentioned earlier, the addition of new pdata fields will cause problems with the DT support already queued for v3.3.

This series should be reworked on top of the DT support which Ben has now queued for v3.3 (my branch: for_3.3/i2c/misc)

Yes will rework it.

Depending on what you plan to do, you'll probably need to coordinate this with Tony also. He's already pulled some of the patches from this series into his i2c branch.

The real question is how you plan to handle the device reset function, given that the driver should compile on a non-OMAP build (meaning that you can't call omap_device*() functions from the driver directly), nor should the driver touch the SYSCONFIG register directly.

Something that puzzle me on that point is most architecture does not use plateform_device and thus does not have any pdata to hack some custom function pointers here an there.
It means that there is probably some better solution to handle that.

AFAIU, the way PPC is handling that is by splitting the driver in a generic part and in a platform specific part.

You can then add any OMAP specific hacks in the OMAP specific part while keeping most of generic code in a generic driver.

In this case, we just have to implement a small shim that will handle the OMAP specific code for the reset. The generic driver will handle the rest.

Maybe I'm missing some important details, but that does seems easily doable for the i2c case.

Quoted 4 lines · Benoit

Something that puzzle me on that point is most architecture does not use plateform_device and thus does not have any pdata to hack some custom function pointers here an there.
It means that there is probably some better solution to handle that.

PCI and USB have their own bus-level device reset functions that are exported from bus code that lives under drivers/. In a similar fashion, we could export omap_device_reset() from our arch/arm/plat-omap code. But that would require that arch/arm/plat-omap and arch/arm/mach-omap2 code to be compiled for any ARM build that includes i2c-omap.c. This would be good to avoid, under the principle that drivers should be buildable for any architecture. (This is also why we wish to remove omap_{read,write}*() and cpu_is_omap() from any drivers which contain those.)

The intention a few years ago was to eventually create a a real omap_bus and omap_device, with the bus/device integration code living under drivers. In such a situation, we'd be able to use something like the PCI/USB approach cleanly, since there wouldn't be dependencies to arch/ code.

Quoted 6 lines

AFAIU, the way PPC is handling that is by splitting the driver in a generic part and in a platform specific part.

You can then add any OMAP specific hacks in the OMAP specific part while keeping most of generic code in a generic driver.

Quoted 5 lines

In this case, we just have to implement a small shim that will handle the OMAP specific code for the reset. The generic driver will handle the rest.

Maybe I'm missing some important details, but that does seems easily doable for the i2c case.

Hmm. How do those drivers call the platform-specific code in a way that allows those drivers to be built for non-PPC platforms?

Quoted 18 lines · Paul Walmsley

Quoted 5 lines · Benoit

Something that puzzle me on that point is most architecture does not use plateform_device and thus does not have any pdata to hack some custom function
pointers here an there.
It means that there is probably some better solution to handle that.

PCI and USB have their own bus-level device reset functions that are exported from bus code that lives under drivers/. In a similar fashion, we could export omap_device_reset() from our arch/arm/plat-omap code. But that would require that arch/arm/plat-omap and arch/arm/mach-omap2 code to be compiled for any ARM build that includes i2c-omap.c. This would be good to avoid, under the principle that drivers should be buildable for any architecture. (This is also why we wish to remove omap_{read,write}*() and cpu_is_omap() from any drivers which contain those.)

Quoted 5 lines

The intention a few years ago was to eventually create a a real omap_bus and omap_device, with the bus/device integration code living under drivers. In such a situation, we'd be able to use something like the PCI/USB approach cleanly, since there wouldn't be dependencies to arch/ code.

For reset, maybe the custom bus is a better approach, since it is something that is handled by the OMAP core infrastructure.
I was referring more to all the other function pointer we have today in driver like MMC, DSS that does not necessarily belong to a OMAP core stuff.

The SYSC register should not accessed in the driver removing the define from the driver.
Also clean up the syscstate from the omap_i2c_dev struct.

Acked-by Santosh Shilimkar <[email protected]>

Signed-off-by Shubhrajyoti D <[email protected]>

---
 drivers/i2c/busses/i2c-omap.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)