Re: Fwd: Hid over I2C and ACPI interaction

2012-07-05 Thread Zhang Rui

  Original Message 
 Subject: Hid over I2C and ACPI interaction
 Date: Wed, 4 Jul 2012 15:46:35 +0200
 From: Benjamin Tissoires benjamin.tissoi...@gmail.com
 To: Jean Delvare kh...@linux-fr.org, Ben Dooks ben-li...@fluff.org, 
 Wolfram 
 Sang w.s...@pengutronix.de, Len Brown l...@kernel.org, 
 linux-a...@vger.kernel.org, linux-i2c@vger.kernel.org, 
 linux-ker...@vger.kernel.org
 CC: Jiri Kosina jkos...@suse.cz, Stéphane Chatty cha...@enac.fr, JJ Ding 
 jj_d...@emc.com.tw
 
 Hi Guys,
 
 I'm the co-author and the maintainer of the hid-multitouch driver. To
 support even more devices, I started the implementation of the HID
 over I2C protocol specification which is introduced by Win8. I'm quite
 comfortable with the hid and the I2C part, but I'm blocked with the
 interaction with the ACPI for the pnp part.
 
 I wanted to have your advice/help on this problem. I've add in the
 recipients list the maintainers of i2c and ACPI, sorry for the noise
 if you don't feel concerned about this.
 
 So, let's go deeper in the problem ;-)
 Microsoft's spec asks the OEM to fill the ACPI DSDT to provide the
 following scope in the ASL layout:
 
  begin of ASL
 Scope (\_SB) {
 //
 //  General Purpose I/O, ports 0...127
 //
 
 Device(HIDI2C_DEVICE1) {
  Name(_ADR,0)
  Name (_HID, MSFT1234”)
  Name (_CID, PNP0C50)
  Name (_UID, 3)
 
  Method(_CRS, 0x0, NotSerialized)
  {
  Name (RBUF, ResourceTemplate ()
  {
  // Address 0x07 on I2C-X (OEM selects this address)
 //IHV SPECIFIC I2C3 = I2C Controller; TGD0 = GPIO Controller;
  I2CSerialBus (0x07, ControllerInitiated,
 10,AddressingMode7Bit, \\_SB.I2C3)
  GpioInt(Level, ActiveLow, Exclusive, PullUp, 0, \\_SB. TGD0,
 0 , ResourceConsumer, , ) {40}
  })
   Return(RBUF)
   }
 
   Method(_DSM, 0x4, NotSerialized)
   {
  // BreakPoint
  Store (Method _DSM begin, Debug)
 
  // DSM UUID
  switch(ToBuffer(Arg0))
  {
  // ACPI DSM UUID for HIDI2C
  case(ToUUID(3CDFF6F7-4267-4555-AD05-B30A3D8938DE))
  {
   // DSM function which returns the HID Descriptor
 Address (skipped)
  }
 
  default
  {
  // No other GUIDs supported
  Return(Buffer(One) { 0x00 })
  }
  }
  }
 }
  end of ASL
 
yep, this is an ACPI enumerated I2C controller.

 Summary:
 - a HID over I2C device has to present the Compatibility ID PNP0C50
 - in the _CRS block, the address, the adapter and the gpioInt are
 defined (or referenced)
 - it presents a Device Specific Method (_DSM) which returns the HID
 Descriptor register address. This register is our entry point for
 retrieving the information about our hid device (so it's mandatory to
 obtain it).
 
 Where am I:
 - I've written a first layer on top of i2c that retrieves the hid
 register (currently the address 0x0001 is hardcoded), then get the
 report desccriptors and the input events, and forward all this stuff
 to the hid layer.
 - It's working with a custom emulated HID over i2c touchpad, while
 waiting for the one a manufacturer should send to me.
 - The detection and the addition to the adapter is done by adding the
 address in the lists and the name through the i2c -detect callback
 (which is not very good, because I don't have the interrupt line
 there).
 - I've written a first acpi implementation that rely on the
 DEVICE_ACPI_HANDLE macro to get the ACPI handle of the device (if
 available).
 - I'm not able to do some tests with the ACPI, as I don't know how to
 implement this DSDT on my computer (I'm missing the I2C part), and the
 manufacturer returned the mainboard with the right DSDT to the OEM.
 
 My questions:
 - will the current acpi implementation handle I2C devices?

you still need to write your own device driver for the device.

 - it seems to me that the .archdata field is left blank during the i2c
 device initialization in all paths I've seen. Is that true?
 - who puts the name int the struct i2c_board_info? (for hot-plugged
 i2c devices).
 
 - finally, what is the best way of handling ACPI for those I2C devices:
1) everything is fine, I should have the ACPI handle in .archdata.
2) someone has to implement the handling of I2C in the pnpACPI layer
 (by adding I2CSerialBus handling and creating there the i2c slave).
3) I should create an acpi driver which handles PNP0C50 and which
 creates the i2c slaves.
 
exactly.

As this I2C controller uses the GPIO interrupt, we need an ACPI GPIO
controller driver for interrupts first.
I already have such a patch in hand, but have not release it for some
reason.
Second, you need to write your own PNP I2C controller driver, to
enumerate the I2C controller via ACPI, AND enumerate the I2C slave
devices under this controller to I2C bus. I also have a similar 

Re: Fwd: Hid over I2C and ACPI interaction

2012-07-05 Thread Zhang Rui
Hah, seems I forgot to reply to Benjamin.

On 四, 2012-07-05 at 15:01 +0800, Zhang Rui wrote:
   Original Message 
  Subject: Hid over I2C and ACPI interaction
  Date: Wed, 4 Jul 2012 15:46:35 +0200
  From: Benjamin Tissoires benjamin.tissoi...@gmail.com
  To: Jean Delvare kh...@linux-fr.org, Ben Dooks ben-li...@fluff.org, 
  Wolfram 
  Sang w.s...@pengutronix.de, Len Brown l...@kernel.org, 
  linux-a...@vger.kernel.org, linux-i2c@vger.kernel.org, 
  linux-ker...@vger.kernel.org
  CC: Jiri Kosina jkos...@suse.cz, Stéphane Chatty cha...@enac.fr, JJ 
  Ding 
  jj_d...@emc.com.tw
  
  Hi Guys,
  
  I'm the co-author and the maintainer of the hid-multitouch driver. To
  support even more devices, I started the implementation of the HID
  over I2C protocol specification which is introduced by Win8. I'm quite
  comfortable with the hid and the I2C part, but I'm blocked with the
  interaction with the ACPI for the pnp part.
  
  I wanted to have your advice/help on this problem. I've add in the
  recipients list the maintainers of i2c and ACPI, sorry for the noise
  if you don't feel concerned about this.
  
  So, let's go deeper in the problem ;-)
  Microsoft's spec asks the OEM to fill the ACPI DSDT to provide the
  following scope in the ASL layout:
  
   begin of ASL
  Scope (\_SB) {
  //
  //  General Purpose I/O, ports 0...127
  //
  
  Device(HIDI2C_DEVICE1) {
   Name(_ADR,0)
   Name (_HID, MSFT1234”)
   Name (_CID, PNP0C50)
   Name (_UID, 3)
  
   Method(_CRS, 0x0, NotSerialized)
   {
   Name (RBUF, ResourceTemplate ()
   {
   // Address 0x07 on I2C-X (OEM selects this address)
  //IHV SPECIFIC I2C3 = I2C Controller; TGD0 = GPIO Controller;
   I2CSerialBus (0x07, ControllerInitiated,
  10,AddressingMode7Bit, \\_SB.I2C3)
   GpioInt(Level, ActiveLow, Exclusive, PullUp, 0, \\_SB. TGD0,
  0 , ResourceConsumer, , ) {40}
   })
Return(RBUF)
}
  
Method(_DSM, 0x4, NotSerialized)
{
   // BreakPoint
   Store (Method _DSM begin, Debug)
  
   // DSM UUID
   switch(ToBuffer(Arg0))
   {  
   // ACPI DSM UUID for HIDI2C
   case(ToUUID(3CDFF6F7-4267-4555-AD05-B30A3D8938DE))
   {
// DSM function which returns the HID Descriptor
  Address (skipped)
   }
  
   default
   {
   // No other GUIDs supported
   Return(Buffer(One) { 0x00 })
   }
   }
   }
  }
   end of ASL
  
 yep, this is an ACPI enumerated I2C controller.
 
  Summary:
  - a HID over I2C device has to present the Compatibility ID PNP0C50
  - in the _CRS block, the address, the adapter and the gpioInt are
  defined (or referenced)
  - it presents a Device Specific Method (_DSM) which returns the HID
  Descriptor register address. This register is our entry point for
  retrieving the information about our hid device (so it's mandatory to
  obtain it).
  
  Where am I:
  - I've written a first layer on top of i2c that retrieves the hid
  register (currently the address 0x0001 is hardcoded), then get the
  report desccriptors and the input events, and forward all this stuff
  to the hid layer.
  - It's working with a custom emulated HID over i2c touchpad, while
  waiting for the one a manufacturer should send to me.
  - The detection and the addition to the adapter is done by adding the
  address in the lists and the name through the i2c -detect callback
  (which is not very good, because I don't have the interrupt line
  there).
  - I've written a first acpi implementation that rely on the
  DEVICE_ACPI_HANDLE macro to get the ACPI handle of the device (if
  available).
  - I'm not able to do some tests with the ACPI, as I don't know how to
  implement this DSDT on my computer (I'm missing the I2C part), and the
  manufacturer returned the mainboard with the right DSDT to the OEM.
  
  My questions:
  - will the current acpi implementation handle I2C devices?
 
 you still need to write your own device driver for the device.
 
  - it seems to me that the .archdata field is left blank during the i2c
  device initialization in all paths I've seen. Is that true?
  - who puts the name int the struct i2c_board_info? (for hot-plugged
  i2c devices).
  
  - finally, what is the best way of handling ACPI for those I2C devices:
 1) everything is fine, I should have the ACPI handle in .archdata.
 2) someone has to implement the handling of I2C in the pnpACPI layer
  (by adding I2CSerialBus handling and creating there the i2c slave).
 3) I should create an acpi driver which handles PNP0C50 and which
  creates the i2c slaves.
  
 exactly.
 
 As this I2C controller uses the GPIO interrupt, we need an ACPI GPIO
 controller driver for interrupts first.
 I already have such a patch in hand, but have not 

Re: [PATCH 7/8 v3] i2c: i801: enable irq for i801 smbus transactions

2012-07-05 Thread Jean Delvare
Hi Daniel,

On Thu, 5 Jul 2012 12:31:11 +0800, Daniel Kurtz wrote:
 On Thu, Jul 5, 2012 at 4:16 AM, Jean Delvare kh...@linux-fr.org wrote:
  You should be able to reproduce this bug by loading i2c-i801 with
  option disable_features=0x0002, assuming your SMBus IRQ is shared.
 
  This leads me to several thoughts (feel free to correct me if I'm
  wrong, I am getting very tired):
 
  1* This must be the reason why a bit was added to the PCI status
 register starting with the ICH5, telling you if an interrupt has been
 delivered to you. Checking for it as you originally did was a good idea
 after all.
 
 Reducing scope to get accepted patches in sooner sounds like a good plan to 
 me.

Agreed.

  2* Is there any reason why you decided to clear the status bits in
 i801_isr(), rather than after wait_event()? I am no expert in
 interrupt handling, I admit, but letting the caller clear the status
 bits would ensure we don't clear status bits when nobody is actually
 waiting to catch them.
 
 BYTE_DONE is cleared to kick off the next byte, and it doesn't
 generate a wait_event, thus it is cleared right in the irq.

No problem with BYTE_DONE.

 The logic for clearing the STATUS_FLAGS was simply that they all
 indicate the end of a transaction, so there won't be any further
 interrupts.  Thus, it is safe to clear it in the irq, since the irq
 will be followed by exactly one wait_event, which can read and process
 saved status.

It is safe if and only if someone is actually listening to the event.
This was not the case for me yesterday. Even if we don't mix interrupts
and polling, i801_isr() might still get called when we aren't
listening. Not only because the IRQ may be shared, but also for events
we don't yet handle, such as an SMBus Alert. Not sure about slave
mode.

  3* Applying this patch (7/8) without the one adding interrupt support
 to i801_block_transaction_byte_by_byte (8/8) is not OK: mixing
 interrupts and polling isn't safe. So unless we implement 1* or 2*,
 both patches would have to be merged before being pushed upstream.
 
  4* Even then, we have to keep in mind that i801_isr() may be called
 before the transaction is finished (if IRQ is shared.) My testing
 has shown that error flags may be raised before the BUSY flag is
 cleared, so we should use in i801_isr() the same strict conditions
 we are now using in the polling code. In other words, if we get an
 interrupt but the BUSY flag is still set, then it's not our
 interrupt and we should ignore it. This applies even if 2* or 3*
 above are implemented, but no longer if 1* is implemented.
 
  5* Your claim that no locking is needed might have to be revisited when
 interrupts are shared.
 
  Implementing 1* has the drawback of limiting interrupt support to ICH5
  and later chips, but I suspect it is the easiest and safest way, so I
  have no objection if you want to do that.
 
 Let's do this first, and then refactor later to add support for
 pre-ICH5 parts, if needed.

OK, fine with me. The only downside is that it excludes my
heavily-shared IRQ test machine, so testing that the shared IRQ case is
properly covered will be a little harder.

 It sounds like supporting the pre-ICH5 parts w/ shared IRQ is not very
 pretty, since we are no longer guaranteed that we get a single
 transaction terminating interrupt.

Indeed. In that case, using interrupts will be much like using polling,
except that status check happens whenever we receive an interrupt,
rather than at fixed time interval.

 Do you still maintain a public git repository?

I never did.

 Have you updated it with the accepted version of the previous cleanup patches?
 I see this one, but it doesn't look updated:
   http://git.kernel.org/?p=linux/kernel/git/jdelvare/staging.git;a=summary

This tree is only for sending patches to Linus. It doesn't reflect the
current status of my working tree.

I am maintaining quilt series for that, which you can find at:
http://khali.linux-fr.org/devel/linux-3/

 I'd like to fix up the interrupt patches per your feedback, but I'm
 not quite sure what the current status is of all the cleanup patches.
 
 In other words, if you push up the complete set of cleanup patches, I
 can then rebase the new consolidated irq patch onto it, and send them
 here for review.

I updated the i2c series this morning, so it's up-to-date.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Hid over I2C and ACPI interaction

2012-07-05 Thread Benjamin Tissoires
Hi,

Many thanks for these information. It seems like I was on the right
track, but I didn't saw the hidden part of the iceberg.
I've already written the i2c slave part (and the acpi handling to get
the HID register by using the DSM should work), but I need now the
whole ACPI pnp drivers...

But without a real ACPI 5.0 mainboard, I think it will be quite
difficult to implement and debug this ACPI stuff.

Cheers,
Benjamin

On Thu, Jul 5, 2012 at 9:20 AM, Zhang Rui rui.zh...@intel.com wrote:
 Hah, seems I forgot to reply to Benjamin.

 On 四, 2012-07-05 at 15:01 +0800, Zhang Rui wrote:
   Original Message 
  Subject: Hid over I2C and ACPI interaction
  Date: Wed, 4 Jul 2012 15:46:35 +0200
  From: Benjamin Tissoires benjamin.tissoi...@gmail.com
  To: Jean Delvare kh...@linux-fr.org, Ben Dooks ben-li...@fluff.org, 
  Wolfram
  Sang w.s...@pengutronix.de, Len Brown l...@kernel.org,
  linux-a...@vger.kernel.org, linux-i2c@vger.kernel.org,
  linux-ker...@vger.kernel.org
  CC: Jiri Kosina jkos...@suse.cz, Stéphane Chatty cha...@enac.fr, JJ 
  Ding
  jj_d...@emc.com.tw
 
  Hi Guys,
 
  I'm the co-author and the maintainer of the hid-multitouch driver. To
  support even more devices, I started the implementation of the HID
  over I2C protocol specification which is introduced by Win8. I'm quite
  comfortable with the hid and the I2C part, but I'm blocked with the
  interaction with the ACPI for the pnp part.
 
  I wanted to have your advice/help on this problem. I've add in the
  recipients list the maintainers of i2c and ACPI, sorry for the noise
  if you don't feel concerned about this.
 
  So, let's go deeper in the problem ;-)
  Microsoft's spec asks the OEM to fill the ACPI DSDT to provide the
  following scope in the ASL layout:
 
   begin of ASL
  Scope (\_SB) {
  //
  //  General Purpose I/O, ports 0...127
  //
 
  Device(HIDI2C_DEVICE1) {
   Name(_ADR,0)
   Name (_HID, MSFT1234”)
   Name (_CID, PNP0C50)
   Name (_UID, 3)
 
   Method(_CRS, 0x0, NotSerialized)
   {
   Name (RBUF, ResourceTemplate ()
   {
   // Address 0x07 on I2C-X (OEM selects this address)
  //IHV SPECIFIC I2C3 = I2C Controller; TGD0 = GPIO Controller;
   I2CSerialBus (0x07, ControllerInitiated,
  10,AddressingMode7Bit, \\_SB.I2C3)
   GpioInt(Level, ActiveLow, Exclusive, PullUp, 0, \\_SB. TGD0,
  0 , ResourceConsumer, , ) {40}
   })
Return(RBUF)
}
 
Method(_DSM, 0x4, NotSerialized)
{
   // BreakPoint
   Store (Method _DSM begin, Debug)
 
   // DSM UUID
   switch(ToBuffer(Arg0))
   {
   // ACPI DSM UUID for HIDI2C
   case(ToUUID(3CDFF6F7-4267-4555-AD05-B30A3D8938DE))
   {
// DSM function which returns the HID Descriptor
  Address (skipped)
   }
 
   default
   {
   // No other GUIDs supported
   Return(Buffer(One) { 0x00 })
   }
   }
   }
  }
   end of ASL
 
 yep, this is an ACPI enumerated I2C controller.

  Summary:
  - a HID over I2C device has to present the Compatibility ID PNP0C50
  - in the _CRS block, the address, the adapter and the gpioInt are
  defined (or referenced)
  - it presents a Device Specific Method (_DSM) which returns the HID
  Descriptor register address. This register is our entry point for
  retrieving the information about our hid device (so it's mandatory to
  obtain it).
 
  Where am I:
  - I've written a first layer on top of i2c that retrieves the hid
  register (currently the address 0x0001 is hardcoded), then get the
  report desccriptors and the input events, and forward all this stuff
  to the hid layer.
  - It's working with a custom emulated HID over i2c touchpad, while
  waiting for the one a manufacturer should send to me.
  - The detection and the addition to the adapter is done by adding the
  address in the lists and the name through the i2c -detect callback
  (which is not very good, because I don't have the interrupt line
  there).
  - I've written a first acpi implementation that rely on the
  DEVICE_ACPI_HANDLE macro to get the ACPI handle of the device (if
  available).
  - I'm not able to do some tests with the ACPI, as I don't know how to
  implement this DSDT on my computer (I'm missing the I2C part), and the
  manufacturer returned the mainboard with the right DSDT to the OEM.
 
  My questions:
  - will the current acpi implementation handle I2C devices?

 you still need to write your own device driver for the device.

  - it seems to me that the .archdata field is left blank during the i2c
  device initialization in all paths I've seen. Is that true?
  - who puts the name int the struct i2c_board_info? (for hot-plugged
  i2c devices).
 
  - finally, what is the best way of handling ACPI for those I2C devices:
 1) everything is 

Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Thomas Petazzoni
Hello Andrew,

Le Tue,  3 Jul 2012 16:22:34 +0200,
Andrew Lunn and...@lunn.ch a écrit :

 Both IRQ and GPIO controllers can now be represented in DT.  The IRQ
 controllers are setup first, and then the GPIO controllers. Interrupts
 for GPIO lines are placed directly after the main interrupts in the
 interrupt space.
 
 Signed-off-by: Andrew Lunn and...@lunn.ch

I have started working on a pinctrl driver for mvebu, which would
handle pin muxing (MPP) + gpio + gpio interrupts.

So far, the pin muxing part is working (needs some polishing, but the
foundation is here), with device tree bindings. I think the pin muxing
could be used for Orion as well.

Now, I'm planning to start working on the gpio + gpio interrupts parts
of the driver, and I'm wondering how to interact with your work on the
matter. My understanding is that the new way of doing a pinmux+gpio
driver is to implement it in drivers/pinctrl/, which I have started
doing.

Should I continue working on a drivers/pinctrl/ driver for mvebu for
Armada 370/XP, and then we see together if it makes sense to extend to
Orion, and if so, what changes are needed?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Andrew Lunn
On Thu, Jul 05, 2012 at 11:02:51AM +0200, Thomas Petazzoni wrote:
 Hello Andrew,
 
 Le Tue,  3 Jul 2012 16:22:34 +0200,
 Andrew Lunn and...@lunn.ch a ??crit :
 
  Both IRQ and GPIO controllers can now be represented in DT.  The IRQ
  controllers are setup first, and then the GPIO controllers. Interrupts
  for GPIO lines are placed directly after the main interrupts in the
  interrupt space.
  
  Signed-off-by: Andrew Lunn and...@lunn.ch
 
 I have started working on a pinctrl driver for mvebu, which would
 handle pin muxing (MPP) + gpio + gpio interrupts.

Hi Thomas

You are not the only one working in this area. Arnaud Patard said he
was look at this as well. 

 So far, the pin muxing part is working (needs some polishing, but the
 foundation is here), with device tree bindings. I think the pin muxing
 could be used for Orion as well.

Great. This is one of the big things which we are missing when moving
a system over the DT. Being able to describe this in DT in a
standardized way is very much welcome.

 Now, I'm planning to start working on the gpio + gpio interrupts parts
 of the driver, and I'm wondering how to interact with your work on the
 matter.

I've publicly said, what i have is enough to get it working, but i
know its not the end solution. What i have is enough that gpio-key,
gpio-led, as described in DT, works. Boards can start using these and
as far as i can tell, the binding should not need to change. The GPIO
controller binding should also be sufficiently generic that it should
also not need changes when we replace the driver with pinctrl.

The biggest problem i had, is the interaction between generic chip
interrupts and irqdomain. There has been work to integrate the two,
but its stalled. Either the work needs restarting and completing, or
you need to throw away the use of generic interrupt so that you can
use irqdomain linear. IMHO, throwing away generic interrupt is the
wrong way.

The other thing you need to keep in mind is the namespace issues
between normal interrupts and GPIO interrupts. The way my patch works
is that normal interrupts are setup first, counting how many have been
created. Then GPIO interrupts are added, starting off where the normal
interrupts ended. Since 370/XP uses different interrupt code, you need
to think how to solve this issue.

 My understanding is that the new way of doing a pinmux+gpio
 driver is to implement it in drivers/pinctrl/, which I have started
 doing.

Great.
 
 Should I continue working on a drivers/pinctrl/ driver for mvebu for
 Armada 370/XP, and then we see together if it makes sense to extend to
 Orion, and if so, what changes are needed?

I know Marvell has contracted you to port to Armada 370/XP, not mvebu,
aka all chips which might be able to share this code. This IMHO is
wrong. So i personally would put the kirkwood and the Armada 370/XP
data sheets next to each other, and from the beginning on write the
driver so it supports them all. This assumes the ASIC engineers have
not radically changed anything... Please also try to stick to the DT
binding i've used.

If however, you have to focus solely on Armada 370/XP, then the
community can work on making your code generic at a late date.

  Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Thomas Petazzoni
Hello,

Le Thu, 5 Jul 2012 11:48:24 +0200,
Andrew Lunn and...@lunn.ch a écrit :

 Hi Thomas
 
 You are not the only one working in this area. Arnaud Patard said he
 was look at this as well. 

Ok, I added Arnaud in the Cc list. Arnaud, what is the status of you
work?

  So far, the pin muxing part is working (needs some polishing, but the
  foundation is here), with device tree bindings. I think the pin muxing
  could be used for Orion as well.
 
 Great. This is one of the big things which we are missing when moving
 a system over the DT. Being able to describe this in DT in a
 standardized way is very much welcome.

For the DT binding, I took my inspiration from the MXS and Samsung
pinctrl drivers, as those platforms also have independent pin muxing
(while some platforms such as Tegra, have hardware pin groups).

The .dtsi side looks like:

pinctrl@d0018000 {
   compatible = marvell,mv78230-pinmux;
   reg = 0xd0018000 0x38;


   uart_2: uart-2 {
marvell,pins = 42 43;
marvell,pins-function = 1;
   };

   uart_3: uart-3 {
marvell,pins = 44 45;
marvell,pins-function = 2;
   };

   uart_0_control: uart-0-control {
marvell,pins = 42 43;
marvell,pins-function = 2;
   };

   uart_1_control: uart-1-control {
marvell,pins = 46 47;
marvell,pins-function = 2;
   };

   uart_2_control: uart-2-control {
marvell,pins = 44 45;
marvell,pins-function = 1;
   };

   uart_3_control: uart-3-control {
marvell,pins = 46 47;
marvell,pins-function = 1;
   };

};

This defines the various pin muxing options, and then when
instantiating a device, you do:

   test@12 {
   compatible = marvell,testdrv;
   pinctrl-names = default;
   pinctrl-0 = uart_0_control;
   };

(This is with a dummy test driver that I'm using for testing). Then
the driver can use the pinctrl API to get the proper muxing for its
pins.

  Now, I'm planning to start working on the gpio + gpio interrupts parts
  of the driver, and I'm wondering how to interact with your work on the
  matter.
 
 I've publicly said, what i have is enough to get it working, but i
 know its not the end solution. What i have is enough that gpio-key,
 gpio-led, as described in DT, works. Boards can start using these and
 as far as i can tell, the binding should not need to change. The GPIO
 controller binding should also be sufficiently generic that it should
 also not need changes when we replace the driver with pinctrl.
 
 The biggest problem i had, is the interaction between generic chip
 interrupts and irqdomain. There has been work to integrate the two,
 but its stalled. Either the work needs restarting and completing, or
 you need to throw away the use of generic interrupt so that you can
 use irqdomain linear. IMHO, throwing away generic interrupt is the
 wrong way.
 
 The other thing you need to keep in mind is the namespace issues
 between normal interrupts and GPIO interrupts. The way my patch works
 is that normal interrupts are setup first, counting how many have been
 created. Then GPIO interrupts are added, starting off where the normal
 interrupts ended. Since 370/XP uses different interrupt code, you need
 to think how to solve this issue.

Ok, thanks for those hints.

  Should I continue working on a drivers/pinctrl/ driver for mvebu for
  Armada 370/XP, and then we see together if it makes sense to extend to
  Orion, and if so, what changes are needed?
 
 I know Marvell has contracted you to port to Armada 370/XP, not mvebu,
 aka all chips which might be able to share this code. This IMHO is
 wrong. So i personally would put the kirkwood and the Armada 370/XP
 data sheets next to each other, and from the beginning on write the
 driver so it supports them all. This assumes the ASIC engineers have
 not radically changed anything... Please also try to stick to the DT
 binding i've used.

I've looked at the datasheet for the 88F6281, and the GPIO registers
look fairly similar with Armada 370/XP, except that:

 * On 88F6281, there are two GPIO banks, while 

Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Thomas Petazzoni
Le Thu, 05 Jul 2012 12:11:40 +0200,
Arnaud Patard (Rtp) arnaud.pat...@rtp-net.org a écrit :

  You are not the only one working in this area. Arnaud Patard said he
  was look at this as well. 
 
 yeah, but tbh I've not made anything yet. If Thomas has already some
 code for it, we should try to make it generic so as to use it on
 armada xp and orion platforms.

The MPP registers are identical on Armada XP/370 and 88F6281 (not sure
which other SoC datasheet I should be checking). Basically, it's just a
range of contiguous registers, with 4 bits per pin to select the
function.

So my pinmux driver should simply work as is for Orion as well. The
only difference between platforms is the number of MPP pins that are
available, but this number also varies between versions of Armada XP
and Armada 370, so I already support this in the driver.

However, using the pinmux driver will require changes in the device
drivers, so that they request the pinmux for their devices.

  I know Marvell has contracted you to port to Armada 370/XP, not mvebu,
  aka all chips which might be able to share this code. This IMHO is
  wrong. So i personally would put the kirkwood and the Armada 370/XP
  data sheets next to each other, and from the beginning on write the
  driver so it supports them all. This assumes the ASIC engineers have
  not radically changed anything... Please also try to stick to the DT
  binding i've used.
 
 agreed. it should be written with all platforms in mind, as long as
 they're compatible. It can only make things better imho.

Certainly, especially for MPP/GPIO that are definitely very similar.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Rtp
Andrew Lunn and...@lunn.ch writes:

Hi,

 On Thu, Jul 05, 2012 at 11:02:51AM +0200, Thomas Petazzoni wrote:
 Hello Andrew,
 
 Le Tue,  3 Jul 2012 16:22:34 +0200,
 Andrew Lunn and...@lunn.ch a ??crit :
 
  Both IRQ and GPIO controllers can now be represented in DT.  The IRQ
  controllers are setup first, and then the GPIO controllers. Interrupts
  for GPIO lines are placed directly after the main interrupts in the
  interrupt space.
  
  Signed-off-by: Andrew Lunn and...@lunn.ch
 
 I have started working on a pinctrl driver for mvebu, which would
 handle pin muxing (MPP) + gpio + gpio interrupts.

 Hi Thomas

 You are not the only one working in this area. Arnaud Patard said he
 was look at this as well. 

yeah, but tbh I've not made anything yet. If Thomas has already some
code for it, we should try to make it generic so as to use it on
armada xp and orion platforms.


 So far, the pin muxing part is working (needs some polishing, but the
 foundation is here), with device tree bindings. I think the pin muxing
 could be used for Orion as well.

 Great. This is one of the big things which we are missing when moving
 a system over the DT. Being able to describe this in DT in a
 standardized way is very much welcome.

 Now, I'm planning to start working on the gpio + gpio interrupts parts
 of the driver, and I'm wondering how to interact with your work on the
 matter.

 I've publicly said, what i have is enough to get it working, but i
 know its not the end solution. What i have is enough that gpio-key,
 gpio-led, as described in DT, works. Boards can start using these and
 as far as i can tell, the binding should not need to change. The GPIO
 controller binding should also be sufficiently generic that it should
 also not need changes when we replace the driver with pinctrl.

 The biggest problem i had, is the interaction between generic chip
 interrupts and irqdomain. There has been work to integrate the two,
 but its stalled. Either the work needs restarting and completing, or
 you need to throw away the use of generic interrupt so that you can
 use irqdomain linear. IMHO, throwing away generic interrupt is the
 wrong way.

 The other thing you need to keep in mind is the namespace issues
 between normal interrupts and GPIO interrupts. The way my patch works
 is that normal interrupts are setup first, counting how many have been
 created. Then GPIO interrupts are added, starting off where the normal
 interrupts ended. Since 370/XP uses different interrupt code, you need
 to think how to solve this issue.

 My understanding is that the new way of doing a pinmux+gpio
 driver is to implement it in drivers/pinctrl/, which I have started
 doing.

 Great.
  
 Should I continue working on a drivers/pinctrl/ driver for mvebu for
 Armada 370/XP, and then we see together if it makes sense to extend to
 Orion, and if so, what changes are needed?

 I know Marvell has contracted you to port to Armada 370/XP, not mvebu,
 aka all chips which might be able to share this code. This IMHO is
 wrong. So i personally would put the kirkwood and the Armada 370/XP
 data sheets next to each other, and from the beginning on write the
 driver so it supports them all. This assumes the ASIC engineers have
 not radically changed anything... Please also try to stick to the DT
 binding i've used.

agreed. it should be written with all platforms in mind, as long as
they're compatible. It can only make things better imho.

Arnaud
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Andrew Lunn
 I've looked at the datasheet for the 88F6281, and the GPIO registers
 look fairly similar with Armada 370/XP, except that:
 
  * On 88F6281, there are two GPIO banks, while on Armada 370/XP it's a
continuous range of registers that control the GPIOs. But it can be
assimilated as one bank handling more than 32 GPIOs. So shouldn't be
a big problem to handle.

The number of banks varies. Orion5x has one, Dove has three, etc.

 
  * The Armada 370/XP have additional set/clear registers that allow to
modify the GPIO states without having to do a read/mask/write
sequence, which is nicer on SMP. I guess we can handle that as a
variant inside the driver without too much problem.
 
 Marvell has given me a 88F6282 eval board, so I guess they are fine
 with me spending some time ensuring that the new code works properly on
 the previous platforms, especially in this case where the amount of
 work does not seem to be enormous to keep the compatibility.

That is good to hear.

Thanks
Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/8 v3] i2c: i801: enable irq for i801 smbus transactions

2012-07-05 Thread Jean Delvare
On Thu, 5 Jul 2012 10:10:15 +0200, Jean Delvare wrote:
 On Thu, 5 Jul 2012 12:31:11 +0800, Daniel Kurtz wrote:
  On Thu, Jul 5, 2012 at 4:16 AM, Jean Delvare kh...@linux-fr.org wrote:
   Implementing 1* has the drawback of limiting interrupt support to ICH5
   and later chips, but I suspect it is the easiest and safest way, so I
   have no objection if you want to do that.
  
  Let's do this first, and then refactor later to add support for
  pre-ICH5 parts, if needed.
 
 OK, fine with me. The only downside is that it excludes my
 heavily-shared IRQ test machine, so testing that the shared IRQ case is
 properly covered will be a little harder.

Actually, no problem there: I can reproduce the issue just fine on my
ICH5 system, which shares an IRQ between the sound chip and the SMBus
controller. So I can use that system to test the updated code too.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Rtp
Thomas Petazzoni thomas.petazz...@free-electrons.com writes:

 Le Thu, 05 Jul 2012 12:11:40 +0200,
 Arnaud Patard (Rtp) arnaud.pat...@rtp-net.org a écrit :

  You are not the only one working in this area. Arnaud Patard said he
  was look at this as well. 
 
 yeah, but tbh I've not made anything yet. If Thomas has already some
 code for it, we should try to make it generic so as to use it on
 armada xp and orion platforms.

 The MPP registers are identical on Armada XP/370 and 88F6281 (not sure
 which other SoC datasheet I should be checking). Basically, it's just a
 range of contiguous registers, with 4 bits per pin to select the
 function.


iirc, other SoCs are similar. The small exception being dove I
guess. Dove has a 3rd gpo [the gpios are output only] bank but to be
used as gpio require that a special bit is set and it's for all gpios of
this bank. You'll find this bit in the general mpp configuration
register if you look at the datasheet.


 So my pinmux driver should simply work as is for Orion as well. The
 only difference between platforms is the number of MPP pins that are
 available, but this number also varies between versions of Armada XP
 and Armada 370, so I already support this in the driver.


Are there some output-only gpio on armada xp/370 like on kirkwood/dove ?


Arnaud
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] mfd: support 88pm80x in 80x driver

2012-07-05 Thread Qiao Zhou
88PM800 and 88PM805 are two discrete chips used for power management.
Hardware designer can use them together or only one of them according
to requirement.

88pm80x_i2c.c provides common i2c driver handling for both 800 and
805, such as i2c_driver init, regmap init, read/write api etc.

88pm800_core.c handles specifically for 800, such as chip init, irq
init/handle, mfd device register, including rtc, onkey, regulator(
to be add later) etc. besides that, 800 has three i2c device, one
regular i2c client, two other i2c dummy for gpadc and power purpose.

88pm805_core.c handles specifically for 805, such as chip init, irq
init/handle, mfd device register, including codec, headset/mic detect
etc.

the i2c operation of both 800 and 805 are via regmap, and 88pm80x-i2c
exported a group of r/w bulk r/w and bits set API for facility.

Signed-off-by: Qiao Zhou zhouq...@marvell.com
---
 drivers/mfd/88pm800-core.c  |  499 
 drivers/mfd/88pm805-core.c  |  282 +++
 drivers/mfd/88pm80x-i2c.c   |  117 ++
 drivers/mfd/Kconfig |   24 ++
 drivers/mfd/Makefile|5 +
 include/linux/mfd/88pm80x.h |  533 +++
 6 files changed, 1460 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/88pm800-core.c
 create mode 100644 drivers/mfd/88pm805-core.c
 create mode 100644 drivers/mfd/88pm80x-i2c.c
 create mode 100644 include/linux/mfd/88pm80x.h

diff --git a/drivers/mfd/88pm800-core.c b/drivers/mfd/88pm800-core.c
new file mode 100644
index 000..6a65ab2
--- /dev/null
+++ b/drivers/mfd/88pm800-core.c
@@ -0,0 +1,499 @@
+/*
+ * Base driver for Marvell 88PM800
+ *
+ * Copyright (C) 2012 Marvell International Ltd.
+ * Haojian Zhuang haojian.zhu...@marvell.com
+ * Joseph(Yossi) Hanin yha...@marvell.com
+ * Qiao Zhou zhouq...@marvell.com
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file COPYING in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/i2c.h
+#include linux/irq.h
+#include linux/interrupt.h
+#include linux/platform_device.h
+#include linux/mfd/core.h
+#include linux/mfd/88pm80x.h
+#include linux/slab.h
+
+static const struct i2c_device_id pm80x_id_table[] = {
+   {88PM800, CHIP_PM800},
+   {88PM805, CHIP_PM805},
+};
+MODULE_DEVICE_TABLE(i2c, pm80x_id_table);
+
+static struct resource rtc_resources[] = {
+   {
+.name = 88pm80x-rtc,
+.start = PM800_IRQ_RTC,
+.end = PM800_IRQ_RTC,
+.flags = IORESOURCE_IRQ,
+},
+};
+
+static struct mfd_cell rtc_devs[] = {
+   {
+.name = 88pm80x-rtc,
+.num_resources = ARRAY_SIZE(rtc_resources),
+.resources = rtc_resources[0],
+.id = -1,
+},
+};
+
+static struct resource onkey_resources[] = {
+   {
+.name = 88pm80x-onkey,
+.start = PM800_IRQ_ONKEY,
+.end = PM800_IRQ_ONKEY,
+.flags = IORESOURCE_IRQ,
+},
+};
+
+static struct mfd_cell onkey_devs[] = {
+   {
+.name = 88pm80x-onkey,
+.num_resources = 1,
+.resources = onkey_resources[0],
+.id = -1,
+},
+};
+
+static const struct regmap_irq pm800_irqs[] = {
+   /* INT0 */
+   [PM800_IRQ_ONKEY] = {
+   .mask = PM800_ONKEY_INT_ENA1,
+   },
+   [PM800_IRQ_EXTON] = {
+   .mask = PM800_EXTON_INT_ENA1,
+   },
+   [PM800_IRQ_CHG] = {
+   .mask = PM800_CHG_INT_ENA1,
+   },
+   [PM800_IRQ_BAT] = {
+   .mask = PM800_BAT_INT_ENA1,
+   },
+   [PM800_IRQ_RTC] = {
+   .mask = PM800_RTC_INT_ENA1,
+   },
+   [PM800_IRQ_CLASSD] = {
+   .mask = PM800_CLASSD_OC_INT_ENA1,
+   },
+   /* INT1 */
+   [PM800_IRQ_VBAT] = {
+   .reg_offset = 1,
+   .mask = PM800_VBAT_INT_ENA2,
+   },
+   [PM800_IRQ_VSYS] = {
+   .reg_offset = 1,
+   .mask = PM800_VSYS_INT_ENA2,
+   },
+   [PM800_IRQ_VCHG] = {
+   .reg_offset = 1,
+   .mask = PM800_VCHG_INT_ENA2,
+   },
+   [PM800_IRQ_TINT] = {
+   .reg_offset = 1,
+   .mask = PM800_TINT_INT_ENA2,
+   },
+   /* INT2 */
+   [PM800_IRQ_GPADC0] = {
+   .reg_offset = 2,
+   .mask = PM800_GPADC0_INT_ENA3,
+   },
+   [PM800_IRQ_GPADC1] = {
+   

[PATCH 0/4 V3] add 88pm80x mfd driver

2012-07-05 Thread Qiao Zhou
change log [v3-v2]:
1, dynamically get the irq_base, for both regmap_add_irq_chip and
mfd_add_devices. add pm80x_request_irq  pm80x_free_irq for sub-driver
facility, and modify related irq thread operation. remove irq_base member
from 80x_chip  platform data.
2, split 88pm80x.o into 3 separate modules.
3, remove the 88pm80x r/w API, and directly use open-coded regmap api.
4, minor change: move pm80x_id_table from 80x-i2c.c to 800/805-core.c,
exported pm80x_init, pm80x_deinit, and pm80x_bulk_read, add callback in
pdata.

change log [v2-v1]:
1, split 88pm80x-core.c into 88pm800-core.c and 88pm805.c, per Arnd's
suggestion. after the re-arch, 88pm80x-i2c handles the 800  805 common
parts, while 800-core.c  805-core.c handle the specific parts in each
chip.
2, add details about the workaround adding a i2c companion between 800 
805, and make a separate patch for it, per Arnd's suggestion.
3, remove callback in pdata. but still keep the pdata currently.
4, only keep necessary register in 88pm80x.h, including registers for
regulator/rtc/onkey/power/codec etc, and remove other registers from global
visibility.
5, exported r/w API which requires regmap handle. as currently the pm800
chip has 3 i2c device, only passing a pm80x_chip info can't ensure r/w the
register in correct i2c device.

change log [v1]:
1, pm800 and pm805 are decoupled and probed separately;
2, re-used the most of API for pm800 and pm805 per Arnd's comments;
3, use regmap_irq, instead of previous 88pm80x_irq_data per Mark's comments.
use regmap_add_irq_chip, and remove previous 88pm80x irq handling.
4, remove callback function in rtc pdata per Arnd's comments.
5, updated some coding style issue.

Qiao Zhou (4):
  mfd: support 88pm80x in 80x driver
  mfd: workaround: add companion chip in 88pm80x
  rtc: add rtc support to 88PM80X PMIC
  input: add onkey support to 88PM80X PMIC

 drivers/input/misc/88pm80x_onkey.c |  190 +
 drivers/input/misc/Kconfig |   10 +
 drivers/input/misc/Makefile|1 +
 drivers/mfd/88pm800-core.c |  499 +
 drivers/mfd/88pm805-core.c |  282 +++
 drivers/mfd/88pm80x-i2c.c  |  145 ++
 drivers/mfd/Kconfig|   24 ++
 drivers/mfd/Makefile   |5 +
 drivers/rtc/Kconfig|   10 +
 drivers/rtc/Makefile   |1 +
 drivers/rtc/rtc-88pm80x.c  |  366 
 include/linux/mfd/88pm80x.h|  534 
 12 files changed, 2067 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/88pm80x_onkey.c
 create mode 100644 drivers/mfd/88pm800-core.c
 create mode 100644 drivers/mfd/88pm805-core.c
 create mode 100644 drivers/mfd/88pm80x-i2c.c
 create mode 100644 drivers/rtc/rtc-88pm80x.c
 create mode 100644 include/linux/mfd/88pm80x.h

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] mfd: workaround: add companion chip in 88pm80x

2012-07-05 Thread Qiao Zhou
in hw design, 800 is mainly for pmic control, while 805 for audio.
but there are 3 registers which controls class D speaker property,
and they are defined in 800 i2c client domain. so 805 codec driver
needs to use 800 i2c client to access class D speaker reg for
audio path management. so add this workaround for the purpose to
let 805 access 800 i2c in some scenario.

Signed-off-by: Qiao Zhou zhouq...@marvell.com
---
 drivers/mfd/88pm80x-i2c.c   |   28 
 include/linux/mfd/88pm80x.h |1 +
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/88pm80x-i2c.c b/drivers/mfd/88pm80x-i2c.c
index 5fc0c9c..ecf93f0 100644
--- a/drivers/mfd/88pm80x-i2c.c
+++ b/drivers/mfd/88pm80x-i2c.c
@@ -19,6 +19,12 @@
 #include linux/uaccess.h
 #include linux/err.h
 
+/*
+ * workaround: some registers needed by pm805 are defined in pm800, so
+ * need to use this global variable to maintain the relation between
+ * pm800 and pm805. would remove it after HW chip fixes the issue.
+ */
+static struct pm80x_chip *g_pm80x_chip;
 
 const struct regmap_config pm80x_regmap_config = {
.reg_bits = 8,
@@ -62,6 +68,19 @@ int __devinit pm80x_init(struct i2c_client *client,
 
device_init_wakeup(client-dev, 1);
 
+   /*
+* workaround: set g_pm80x_chip to the first probed chip. if the
+* second chip is probed, just point to the companion to each
+* other so that pm805 can access those specific register. would
+* remove it after HW chip fixes the issue.
+*/
+   if (!g_pm80x_chip)
+   g_pm80x_chip = chip;
+   else {
+   chip-companion = g_pm80x_chip-client;
+   g_pm80x_chip-companion = chip-client;
+   }
+
return 0;
 
 err_chip_id:
@@ -76,6 +95,15 @@ int __devexit pm80x_deinit(struct i2c_client *client)
 {
struct pm80x_chip *chip = i2c_get_clientdata(client);
 
+   /*
+* workaround: clear the dependency between pm800 and pm805.
+* would remove it after HW chip fixes the issue.
+*/
+   if (g_pm80x_chip-companion)
+   g_pm80x_chip-companion = NULL;
+   else
+   g_pm80x_chip = NULL;
+
regmap_exit(chip-regmap);
devm_kfree(client-dev, chip);
 
diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
index 687f296..6c91144 100644
--- a/include/linux/mfd/88pm80x.h
+++ b/include/linux/mfd/88pm80x.h
@@ -487,6 +487,7 @@ struct pm80x_chip {
struct pm80x_subchip *subchip;
struct device *dev;
struct i2c_client *client;
+   struct i2c_client *companion;
struct regmap *regmap;
struct regmap_irq_chip *regmap_irq_chip;
struct regmap_irq_chip_data *irq_data;
-- 
1.7.0.4

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] rtc: add rtc support to 88PM80X PMIC

2012-07-05 Thread Qiao Zhou
add rtc driver for MARVELL 88PM80X PMIC and enable rtc function.

Signed-off-by: Qiao Zhou zhouq...@marvell.com
---
 drivers/rtc/Kconfig   |   10 ++
 drivers/rtc/Makefile  |1 +
 drivers/rtc/rtc-88pm80x.c |  366 +
 3 files changed, 377 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-88pm80x.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 08cbdb9..f3b49f8 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -135,6 +135,16 @@ config RTC_DRV_88PM860X
  This driver can also be built as a module. If so, the module
  will be called rtc-88pm860x.
 
+config RTC_DRV_88PM80X
+   tristate Marvell 88PM80x
+   depends on RTC_CLASS  I2C  MFD_88PM80X
+   help
+ If you say yes here you get support for RTC function in Marvell
+ 88PM80x chips.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-88pm80x.
+
 config RTC_DRV_DS1307
tristate Dallas/Maxim DS1307/37/38/39/40, ST M41T00, EPSON RX-8025
help
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 2973921..0d5b2b6 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -16,6 +16,7 @@ rtc-core-$(CONFIG_RTC_INTF_SYSFS) += rtc-sysfs.o
 # Keep the list ordered.
 
 obj-$(CONFIG_RTC_DRV_88PM860X)  += rtc-88pm860x.o
+obj-$(CONFIG_RTC_DRV_88PM80X)  += rtc-88pm80x.o
 obj-$(CONFIG_RTC_DRV_AB3100)   += rtc-ab3100.o
 obj-$(CONFIG_RTC_DRV_AB8500)   += rtc-ab8500.o
 obj-$(CONFIG_RTC_DRV_AT32AP700X)+= rtc-at32ap700x.o
diff --git a/drivers/rtc/rtc-88pm80x.c b/drivers/rtc/rtc-88pm80x.c
new file mode 100644
index 000..d443157
--- /dev/null
+++ b/drivers/rtc/rtc-88pm80x.c
@@ -0,0 +1,366 @@
+/*
+ * Real Time Clock driver for Marvell 88PM80x PMIC
+ *
+ * Copyright (c) 2012 Marvell International Ltd.
+ *  Wenzeng Chenw...@marvell.com
+ *  Qiao Zhou zhouq...@marvell.com
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file COPYING in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/slab.h
+#include linux/regmap.h
+#include linux/mfd/core.h
+#include linux/mfd/88pm80x.h
+#include linux/rtc.h
+
+struct pm80x_rtc_info {
+   struct pm80x_chip *chip;
+   struct regmap *map;
+   struct rtc_device *rtc_dev;
+   struct device *dev;
+   struct delayed_work calib_work;
+
+   int irq;
+   int vrtc;
+};
+
+static int pm80x_rtc_read_time(struct device *dev, struct rtc_time *tm);
+
+static irqreturn_t rtc_update_handler(int irq, void *data)
+{
+   struct pm80x_rtc_info *info = (struct pm80x_rtc_info *)data;
+   int mask;
+
+   mask = PM800_ALARM | PM800_ALARM_WAKEUP;
+   regmap_update_bits(info-map, PM800_RTC_CONTROL, mask | PM800_ALARM1_EN,
+  mask);
+   rtc_update_irq(info-rtc_dev, 1, RTC_AF);
+   return IRQ_HANDLED;
+}
+
+static int pm80x_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+   struct pm80x_rtc_info *info = dev_get_drvdata(dev);
+
+   if (enabled)
+   regmap_update_bits(info-map, PM800_RTC_CONTROL,
+  PM800_ALARM1_EN, PM800_ALARM1_EN);
+   else
+   regmap_update_bits(info-map, PM800_RTC_CONTROL,
+  PM800_ALARM1_EN, 0);
+   return 0;
+}
+
+/*
+ * Calculate the next alarm time given the requested alarm time mask
+ * and the current time.
+ */
+static void rtc_next_alarm_time(struct rtc_time *next, struct rtc_time *now,
+   struct rtc_time *alrm)
+{
+   unsigned long next_time;
+   unsigned long now_time;
+
+   next-tm_year = now-tm_year;
+   next-tm_mon = now-tm_mon;
+   next-tm_mday = now-tm_mday;
+   next-tm_hour = alrm-tm_hour;
+   next-tm_min = alrm-tm_min;
+   next-tm_sec = alrm-tm_sec;
+
+   rtc_tm_to_time(now, now_time);
+   rtc_tm_to_time(next, next_time);
+
+   if (next_time  now_time) {
+   /* Advance one day */
+   next_time += 60 * 60 * 24;
+   rtc_time_to_tm(next_time, next);
+   }
+}
+
+static int pm80x_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+   struct pm80x_rtc_info *info = dev_get_drvdata(dev);
+   unsigned char buf[4];
+   unsigned long ticks, base, data;
+   regmap_raw_read(info-map, 

[PATCH 4/4] input: add onkey support to 88PM80X PMIC

2012-07-05 Thread Qiao Zhou
add onkey support to MARVELL 88PM80X PMIC.

Signed-off-by: Qiao Zhou zhouq...@marvell.com
---
 drivers/input/misc/88pm80x_onkey.c |  190 
 drivers/input/misc/Kconfig |   10 ++
 drivers/input/misc/Makefile|1 +
 3 files changed, 201 insertions(+), 0 deletions(-)
 create mode 100644 drivers/input/misc/88pm80x_onkey.c

diff --git a/drivers/input/misc/88pm80x_onkey.c 
b/drivers/input/misc/88pm80x_onkey.c
new file mode 100644
index 000..b202448
--- /dev/null
+++ b/drivers/input/misc/88pm80x_onkey.c
@@ -0,0 +1,190 @@
+/*
+ * 88pm80x_onkey.c - Marvell 88PM80x ONKEY driver
+ *
+ * Copyright (C) 2012 Marvell International Ltd.
+ * Haojian Zhuang haojian.zhu...@marvell.com
+ * Qiao Zhou zhouq...@marvell.com
+ *
+ * This file is subject to the terms and conditions of the GNU General
+ * Public License. See the file COPYING in the main directory of this
+ * archive for more details.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/input.h
+#include linux/interrupt.h
+#include linux/mfd/88pm80x.h
+#include linux/regmap.h
+#include linux/slab.h
+
+#define PM800_LONG_ONKEY_EN(1  0)
+#define PM800_LONG_KEY_DELAY   (8) /* 1 .. 16 seconds */
+#define PM800_LONKEY_PRESS_TIME((PM800_LONG_KEY_DELAY-1)  4)
+#define PM800_LONKEY_PRESS_TIME_MASK   (0xF0)
+#define PM800_SW_PDOWN (1  5)
+
+struct pm80x_onkey_info {
+   struct input_dev *idev;
+   struct pm80x_chip *pm80x;
+   struct regmap *map;
+   int irq;
+};
+
+/* 88PM80x gives us an interrupt when ONKEY is held */
+static irqreturn_t pm80x_onkey_handler(int irq, void *data)
+{
+   struct pm80x_onkey_info *info = data;
+   int ret = 0;
+
+   regmap_read(info-map, PM800_STATUS_1, ret);
+   ret = PM800_ONKEY_STS1;
+
+   input_report_key(info-idev, KEY_POWER, ret);
+   input_sync(info-idev);
+
+   return IRQ_HANDLED;
+}
+
+#ifdef CONFIG_PM
+static int pm80x_onkey_suspend(struct device *dev)
+{
+   struct platform_device *pdev = to_platform_device(dev);
+   struct pm80x_chip *chip = dev_get_drvdata(pdev-dev.parent);
+
+   if (device_may_wakeup(dev))
+   chip-wu_flag |= (1  PM800_IRQ_ONKEY);
+
+   return 0;
+}
+
+static int pm80x_onkey_resume(struct device *dev)
+{
+   struct platform_device *pdev = to_platform_device(dev);
+   struct pm80x_chip *chip = dev_get_drvdata(pdev-dev.parent);
+
+   if (device_may_wakeup(dev))
+   chip-wu_flag = ~(1  PM800_IRQ_ONKEY);
+
+   return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(pm80x_onkey_pm_ops, pm80x_onkey_suspend,
+pm80x_onkey_resume);
+
+static int __devinit pm80x_onkey_probe(struct platform_device *pdev)
+{
+
+   void *chip = dev_get_drvdata(pdev-dev.parent);
+   struct pm80x_onkey_info *info;
+   int ret;
+
+   info =
+   devm_kzalloc(pdev-dev, sizeof(struct pm80x_onkey_info),
+GFP_KERNEL);
+   if (!info)
+   return -ENOMEM;
+
+   info-pm80x = (struct pm80x_chip *)chip;
+
+   info-irq = platform_get_irq(pdev, 0);
+   if (info-irq  0) {
+   dev_err(pdev-dev, No IRQ resource!\n);
+   return -EINVAL;
+   }
+
+   info-map = info-pm80x-regmap;
+   if (!info-map) {
+   dev_err(pdev-dev, no regmap!\n);
+   ret = -EINVAL;
+   goto out;
+   }
+
+   info-idev = input_allocate_device();
+   if (!info-idev) {
+   dev_err(pdev-dev, Failed to allocate input dev\n);
+   ret = -ENOMEM;
+   goto out;
+   }
+
+   info-idev-name = 88pm80x_on;
+   info-idev-phys = 88pm80x_on/input0;
+   info-idev-id.bustype = BUS_I2C;
+   info-idev-dev.parent = pdev-dev;
+   info-idev-evbit[0] = BIT_MASK(EV_KEY);
+   info-idev-keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);
+
+   ret = input_register_device(info-idev);
+   if (ret) {
+   dev_err(pdev-dev, Can't register input device: %d\n, ret);
+   goto out_reg;
+   }
+
+   ret = pm80x_request_irq(info-pm80x, info-irq, pm80x_onkey_handler,
+   IRQF_ONESHOT, onkey, info);
+   if (ret  0) {
+   dev_err(pdev-dev, Failed to request IRQ: #%d: %d\n,
+   info-irq, ret);
+   goto out_irq;
+   }
+
+   

Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Thomas Petazzoni
Le Thu, 05 Jul 2012 12:38:51 +0200,
Arnaud Patard (Rtp) arnaud.pat...@rtp-net.org a écrit :

  The MPP registers are identical on Armada XP/370 and 88F6281 (not sure
  which other SoC datasheet I should be checking). Basically, it's just a
  range of contiguous registers, with 4 bits per pin to select the
  function.
 
 iirc, other SoCs are similar. The small exception being dove I
 guess. Dove has a 3rd gpo [the gpios are output only] bank but to be
 used as gpio require that a special bit is set and it's for all gpios of
 this bank. You'll find this bit in the general mpp configuration
 register if you look at the datasheet.

Ok, this is a bit trickier, but we can probably do something for it as
well.

  So my pinmux driver should simply work as is for Orion as well. The
  only difference between platforms is the number of MPP pins that are
  available, but this number also varies between versions of Armada XP
  and Armada 370, so I already support this in the driver.
 
 Are there some output-only gpio on armada xp/370 like on kirkwood/dove ?

Yes. I am not sure yet how to describe those in the DT, or even if it
is actually useful to describe them. Wouldn't it be simpler to just
leave to the user of the GPIO to use a GPIO that's appropriate for its
usage, i.e not use a GPO when input is needed?

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Andrew Lunn
Hi Thomas

 Yes. I am not sure yet how to describe those in the DT, or even if it
 is actually useful to describe them. Wouldn't it be simpler to just
 leave to the user of the GPIO to use a GPIO that's appropriate for its
 usage, i.e not use a GPO when input is needed?

We assume the hardware designer has got the basic hardware right. Its
not going to work otherwise.

What we are trying to detect is a DT author making a typo, assigning a
gpio-key to a GPO pin, for example. The current MPP scheme will detect
this sort of error and issue a warning to the kernel logs.

 Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] mfd: support 88pm80x in 80x driver

2012-07-05 Thread Arnd Bergmann
On Thursday 05 July 2012, Qiao Zhou wrote:
 +
 +static const struct i2c_device_id pm80x_id_table[] = {
 + {88PM800, CHIP_PM800},
 + {88PM805, CHIP_PM805},
 +};
 +MODULE_DEVICE_TABLE(i2c, pm80x_id_table);

The point of moving the table to the individual drivers was that the
right one can get loaded automatically. This requires of course that
you put only one in there. It really needs to be

static const struct i2c_device_id pm800_id_table[] = {
{88PM800, CHIP_PM800},
};
MODULE_DEVICE_TABLE(i2c, pm800_id_table);

and

static const struct i2c_device_id pm805_id_table[] = {
{88PM805, CHIP_PM805},
};
MODULE_DEVICE_TABLE(i2c, pm805_id_table);

 diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
 index 75f6ed6..22dba98 100644
 --- a/drivers/mfd/Makefile
 +++ b/drivers/mfd/Makefile
 @@ -3,7 +3,12 @@
  #
  
  88pm860x-objs:= 88pm860x-core.o 88pm860x-i2c.o
 +88pm80x-objs := 88pm80x-i2c.o
 +88pm800-objs := 88pm800-core.o
 +88pm805-objs := 88pm805-core.o
  obj-$(CONFIG_MFD_88PM860X)   += 88pm860x.o
 +obj-$(CONFIG_MFD_88PM800)+= 88pm800.o 88pm80x.o
 +obj-$(CONFIG_MFD_88PM805)+= 88pm805.o 88pm80x.o

This can be written much shorter if you leave out the 88pm80?-objs:= lines
and just build each module from one file as in

obj-$(CONFIG_MFD_88PM800)   += 88pm800-core.o 88pm80x-i2c.o

It might make sense to drop the core and i2c postfixes on the names,
your choice.

 diff --git a/include/linux/mfd/88pm80x.h b/include/linux/mfd/88pm80x.h
 new file mode 100644
 index 000..687f296
 --- /dev/null
 +++ b/include/linux/mfd/88pm80x.h

I don't really like repeating myself, but this still contains a lot
of crap that needs to be moved out of this file, into the places
where it's used:

 +enum {
 + /* Procida */
 + PM800_CHIP_A0  = 0x60,
 + PM800_CHIP_A1  = 0x61,
 + PM800_CHIP_B0  = 0x62,
 + PM800_CHIP_C0  = 0x63,
 + PM800_CHIP_END = PM800_CHIP_C0,
 +
 + /* Make sure to update this to the last stepping */
 + PM8XXX_CHIP_END = PM800_CHIP_END
 +};

88pm800-core.c

 +enum {
 + PM800_ID_INVALID,
 + PM800_ID_VIBRATOR,
 + PM800_ID_SOUND,
 + PM800_ID_MAX,
 +};

unused?

 +enum {
 + PM800_ID_BUCK1 = 0,
 + PM800_ID_BUCK2,
 + PM800_ID_BUCK3,
 + PM800_ID_BUCK4,
 + PM800_ID_BUCK5,
 +
 + PM800_ID_LDO1,
 + PM800_ID_LDO2,
 + PM800_ID_LDO3,
 + PM800_ID_LDO4,
 + PM800_ID_LDO5,
 + PM800_ID_LDO6,
 + PM800_ID_LDO7,
 + PM800_ID_LDO8,
 + PM800_ID_LDO9,
 + PM800_ID_LDO10,
 + PM800_ID_LDO11,
 + PM800_ID_LDO12,
 + PM800_ID_LDO13,
 + PM800_ID_LDO14,
 + PM800_ID_LDO15,
 + PM800_ID_LDO16,
 + PM800_ID_LDO17,
 + PM800_ID_LDO18,
 + PM800_ID_LDO19,
 +
 + PM800_ID_RG_MAX,
 +};
 +#define PM800_MAX_REGULATOR  PM800_ID_RG_MAX /* 5 Bucks, 19 LDOs */
 +#define PM800_NUM_BUCK (5)   /*5 Bucks */
 +#define PM800_NUM_LDO (19)   /*19 Bucks */

unused? should probably go into the regulator driver

 +/* 88PM805 Registers */
 +#define PM805_CHIP_ID(0x00)

88pm805-core.c

 +/* Audio */
 +
 +/* 88PM800 registers */
 +enum {
 + PM80X_INVALID_PAGE = 0,
 + PM80X_BASE_PAGE,
 + PM80X_POWER_PAGE,
 + PM80X_GPADC_PAGE,
 + PM80X_TEST_PAGE,
 +};

unused?

 +/* page 0 basic: slave adder 0x60 */
 +
 +/* Interrupt Registers */
 +#define PM800_CHIP_ID(0x00)
 +
 +#define PM800_STATUS_1   (0x01)
 +#define PM800_ONKEY_STS1 (1  0)
 +#define PM800_EXTON_STS1 (1  1)
 +#define PM800_CHG_STS1   (1  2)
 +#define PM800_BAT_STS1   (1  3)
 +#define PM800_VBUS_STS1  (1  4)
 +#define PM800_LDO_PGOOD_STS1 (1  5)
 +#define PM800_BUCK_PGOOD_STS1(1  6)
 +
 +#define PM800_STATUS_2   (0x02)
 +#define PM800_RTC_ALARM_STS2 (1  0)

These can probably stay here.

 +#define PM800_INT_STATUS1(0x05)
 +#define PM800_ONKEY_INT_STS1 (1  0)
 +#define PM800_EXTON_INT_STS1 (1  1)
 +#define PM800_CHG_INT_STS1   (1  2)
 +#define PM800_BAT_INT_STS1   (1  3)
 +#define PM800_RTC_INT_STS1   (1  4)
 +#define PM800_CLASSD_OC_INT_STS1 (1  5)
 ...
 +/* number of INT_ENA  INT_STATUS regs */
 +#define PM800_INT_REG_NUM(4)

all the interrupt handling can go into 88pm800-core.c

 +/* RTC Registers */
 +#define PM800_RTC_CONTROL(0xD0)
 +#define PM800_RTC_COUNTER1   (0xD1)
 +#define PM800_RTC_COUNTER2   (0xD2)
 +#define PM800_RTC_COUNTER3   (0xD3)
 +#define PM800_RTC_COUNTER4   (0xD4)
 +#define PM800_RTC_EXPIRE1_1  (0xD5)
 +#define PM800_RTC_EXPIRE1_2  (0xD6)
 +#define PM800_RTC_EXPIRE1_3  (0xD7)
 +#define PM800_RTC_EXPIRE1_4  (0xD8)
 +#define PM800_RTC_TRIM1  (0xD9)
 +#define PM800_RTC_TRIM2   

Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Sebastian Hesselbarth

On 07/05/2012 01:48 PM, Andrew Lunn wrote:
 Yes. I am not sure yet how to describe those in the DT, or even if it
 is actually useful to describe them. Wouldn't it be simpler to just
 leave to the user of the GPIO to use a GPIO that's appropriate for its
 usage, i.e not use a GPO when input is needed?

 We assume the hardware designer has got the basic hardware right. Its
 not going to work otherwise.

 What we are trying to detect is a DT author making a typo, assigning a
 gpio-key to a GPO pin, for example. The current MPP scheme will detect
 this sort of error and issue a warning to the kernel logs.

The output-only/non-interrupt gpios could be marked as such in DT.
The common code should not install irqs if marked by e.g.
marvell,gpio-no-irq. The gpio driver already fails to set input
direction on output-only pins. This could be marked by e.g.
marvell,gpio-output-only in DT.

Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Arnd Bergmann
On Tuesday 03 July 2012, Andrew Lunn wrote:
 Both IRQ and GPIO controllers can now be represented in DT.  The IRQ
 controllers are setup first, and then the GPIO controllers. Interrupts
 for GPIO lines are placed directly after the main interrupts in the
 interrupt space.

Overall looks very good.

 diff --git a/Documentation/devicetree/bindings/arm/mrvl/intc.txt 
 b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
 index 80b9a94..8927e10 100644
 --- a/Documentation/devicetree/bindings/arm/mrvl/intc.txt
 +++ b/Documentation/devicetree/bindings/arm/mrvl/intc.txt
 @@ -38,3 +38,22 @@ Example:
   reg-names = mux status, mux mask;
   mrvl,intc-nr-irqs = 2;
   };
 +
 +* Marvell Orion Interrupt controller
 +
 +Required properties
 +- compatible :  Should be marvell,orion-intc.
 +- #interrupt-cells: Specifies the number of cells needed to encode an
 +  interrupt source. Supported value is 1.
 +- interrupt-controller : Declare this node to be an interrupt controller.
 +- reg : Interrupt mask address.

I think you should clarify that the reg property can be multiple
4-byte ranges, because that is fairly unusual.

 +#ifdef CONFIG_OF
 +static int __init orion_add_irq_domain(struct device_node *np,
 +struct device_node *interrupt_parent)
 +{
 + int i = 0, irq_gpio;
 + void __iomem *base;
 +
 + do {
 + base = of_iomap(np, i);
 + if (base) {
 + orion_irq_init(i * 32, base);
 + i++;
 + }
 + } while (base);
 +
 + irq_domain_add_legacy(np, i * 32, 0, 0,
 +   irq_domain_simple_ops, NULL);
 +
 + irq_gpio = i * 32;
 + orion_gpio_of_init(irq_gpio);
 +
 + return 0;
 +}
 +
 +static const struct of_device_id orion_irq_match[] = {
 + { .compatible = marvell,orion-intc,
 +   .data = orion_add_irq_domain, },
 + {},
 +};
 +
 +void __init orion_dt_init_irq(void)
 +{
 + of_irq_init(orion_irq_match);
 +}
 +#endif

I'm wondering about this one.  The other platforms usually put the secondary
interrupt controllers into the same match table, while you call 
orion_gpio_of_init
from orion_add_irq_domain. Can you explain why you do this? Does it have
any disadvantages?

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v6 7/7] ARM: davinci: add support for the am1808 based enbw_cmc board

2012-07-05 Thread Heiko Schocher
- AM1808 based board
- 64 MiB DDR ram
- 2 MiB Nor flash
- 128 MiB NAND flash
- use internal RTC
- I2C support
- hwmon lm75 support
- UBI/UBIFS support
- MMC support
- USB OTG support

Signed-off-by: Heiko Schocher h...@denx.de
Cc: linux-arm-ker...@lists.infradead.org
Cc: devicetree-disc...@lists.ozlabs.org
Cc: davinci-linux-open-sou...@linux.davincidsp.com
Cc: linux-...@lists.infradead.org
Cc: linux-i2c@vger.kernel.org
Cc: net...@vger.kernel.org
Cc: David Woodhouse dw...@infradead.org
Cc: Ben Dooks ben-li...@fluff.org
Cc: Wolfram Sang w.s...@pengutronix.de
Cc: Sekhar Nori nsek...@ti.com
Cc: Kevin Hilman khil...@ti.com
Cc: Wolfgang Denk w...@denx.de
Cc: Scott Wood scottw...@freescale.com
Cc: Sylwester Nawrocki s.nawro...@samsung.com

---
- post this board support with USB support, even though
  USB is only working with the 10 ms workaround, posted here:
  http://comments.gmane.org/gmane.linux.usb.general/54505
  I see this issue also on the AM1808 TMDXEXP1808L evalboard.
- MMC and USB are not using OF support yet, ideas how to port
  this are welcome. I need for USB and MMC boards board
  specific callbacks, how to solve this with OF support?

- changes for v2:
  - changes in the nand node due to comments from Scott Wood:
- add ti,davinci- prefix
- Dashes are preferred to underscores
- rename nandflash to nand
- introduce new ti,davinci specific properties for setting
  up ecc_mode, ecc_bits, options and bbt options, instead
  using linux defines
  - changes for i2c due to comments from Sylwester Nawrocki:
- use cell-index instead id
- OF_DEV_AUXDATA in the machine code, instead pre-define
  platform device name
  - add comment from Grant Likely for i2c:
- removed id resp. cell-index completely
- fixed documentation
- use of_match_ptr()
- use devm_kzalloc() for allocating plattform data mem
- fixed a whitespace issue
  - add net comments from Grant Likely:
- add prefix ti,davinci- to davinci specific property names
- remove version property
- use compatible name ti,davinci-dm6460-emac
  - add comment from Grant Likely:
- rename compatible node
- do not use cell-index
- CONFIG_OF required for this board
TODO:
- create a generic board support file, as I got no
  answer to my ping to grant, maybe this could be done
  in a second step?
- changes for v3:
  - add comments from Sergei Shtylyov:
- rename compatible prop to ti,cp_intc
- cp_intc_init now used for Interrupt controller init
- changes for v4:
  add comment from Nori Sekhar:
  - rename davinci emac compatible property to ti,davinci-dm6467-emac
  - remove pinmux-handle property as discussed here:
http://www.spinics.net/lists/arm-kernel/msg175701.html
with Nori Sekhar

- changes for v5:
  add comments from Grant Likely:
  - rename compatible prop to ti,cp-intc

- changes for v6:
  rework this patch, as patch
  ARM: davinci: cp_intc: Add OF support for TI interrupt controller
  was changed from Nori Sekhar on Jul 03, 2012; 9:16pm
  Changes therefore in this patch:

  Call of_irq_init() in the generic DT board file and not in the
  interrupt controller code. See arch/arm/mach-at91/board-dt.c or
  arch/arm/mach-omap2/board-generic.c for examples.

  At this point the question raises, if we should rename this
  board port from arch/arm/mach-davinci/enbw_cmc.c to
  arch/arm/mach-davinci/board-dt.c ? Also the defconfig to
  davinci_of_defconfig ... ? The USB and MMC callbacks are
  currently board specific, but if other boards come in,
  that could be easily adapted for their needs ...

 arch/arm/boot/dts/enbw_cmc.dts  |  183 +++
 arch/arm/configs/enbw_cmc_defconfig |  126 
 arch/arm/mach-davinci/Kconfig   |9 +
 arch/arm/mach-davinci/Makefile  |1 +
 arch/arm/mach-davinci/board-enbw-cmc.c  |  385 +++
 arch/arm/mach-davinci/include/mach/uncompress.h |1 +
 6 files changed, 705 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/enbw_cmc.dts
 create mode 100644 arch/arm/configs/enbw_cmc_defconfig
 create mode 100644 arch/arm/mach-davinci/board-enbw-cmc.c

diff --git a/arch/arm/boot/dts/enbw_cmc.dts b/arch/arm/boot/dts/enbw_cmc.dts
new file mode 100644
index 000..32dc7a9
--- /dev/null
+++ b/arch/arm/boot/dts/enbw_cmc.dts
@@ -0,0 +1,183 @@
+/*
+ * Device Tree for the EnBW CMC plattform
+ *
+ * Copyright 2011 DENX Software Engineering GmbH
+ * Heiko Schocher h...@denx.de
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+/dts-v1/;
+/include/ skeleton.dtsi
+
+/ {
+   model = EnBW CMC;
+   compatible = enbw,cmc;
+
+   aliases {
+   ethernet0 = eth0;
+   };
+
+   arm {
+  

Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Thomas Petazzoni
Hello,

Le Thu, 5 Jul 2012 11:48:24 +0200,
Andrew Lunn and...@lunn.ch a écrit :

 The biggest problem i had, is the interaction between generic chip
 interrupts and irqdomain. There has been work to integrate the two,
 but its stalled. Either the work needs restarting and completing, or
 you need to throw away the use of generic interrupt so that you can
 use irqdomain linear. IMHO, throwing away generic interrupt is the
 wrong way.

Can you expand on why you think it would be wrong to throw away the
usage of irq_chip_generic, compared to implementing directly an
irq_chip?

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Andrew Lunn
  +Required properties
  +- compatible :  Should be marvell,orion-intc.
  +- #interrupt-cells: Specifies the number of cells needed to encode an
  +  interrupt source. Supported value is 1.
  +- interrupt-controller : Declare this node to be an interrupt controller.
  +- reg : Interrupt mask address.
 
 I think you should clarify that the reg property can be multiple
 4-byte ranges, because that is fairly unusual.

O.K. The example is already like this, but i can add some more words.

  +#ifdef CONFIG_OF
  +static int __init orion_add_irq_domain(struct device_node *np,
  +  struct device_node *interrupt_parent)
  +{
  +   int i = 0, irq_gpio;
  +   void __iomem *base;
  +
  +   do {
  +   base = of_iomap(np, i);
  +   if (base) {
  +   orion_irq_init(i * 32, base);
  +   i++;
  +   }
  +   } while (base);
  +
  +   irq_domain_add_legacy(np, i * 32, 0, 0,
  + irq_domain_simple_ops, NULL);
  +
  +   irq_gpio = i * 32;
  +   orion_gpio_of_init(irq_gpio);
  +
  +   return 0;
  +}
  +
  +static const struct of_device_id orion_irq_match[] = {
  +   { .compatible = marvell,orion-intc,
  + .data = orion_add_irq_domain, },
  +   {},
  +};
  +
  +void __init orion_dt_init_irq(void)
  +{
  +   of_irq_init(orion_irq_match);
  +}
  +#endif
 
 I'm wondering about this one.  The other platforms usually put the secondary
 interrupt controllers into the same match table, while you call 
 orion_gpio_of_init
 from orion_add_irq_domain. Can you explain why you do this? Does it have
 any disadvantages?

The issue is knowing what IRQ number to use for the secondary
interrupts.

Orion use generic chip interrupts, both for the main interrupts and
the GPIO interrupts. This does not yet support irq domain, so i have
to layer a legacy domain on top. The legacy domain needs to know the
first IRQ and the number of IRQs. For the primary IRQs that is
easy. However, GPIO IRQ is not so easy, it depends on how many primary
IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood,
64, and mv78xx0 has 96. I need to know this number when adding the
GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in
the orion_add_irq_domain() i have this number to hand. If i used to
entries in the match table, i would have to put this number into some
global variable, or somehow ask the IRQ subsystem what the next free
IRQ number is.

As for disadvantages, humm. Dove has yet more interrupts, from the
PMU. They are currently unsupported in DT. When we add support for the
PMU interrupt controller, we are going to have the same problem, what
IRQ base should it use. Either we extend the chaining, calling
dove_pmu_of_init from orion_gpio_of_init(), where we know the next
free IRQ. Or we find out how to ask the IRQ subsystem for the next
available. Better still, the work to make generic chip interrupts irq
domain aware would get completed, and we can swap all this code to irq
domain linear and this whole probable probably goes away.

Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] i2c i.MX: Fix divider table

2012-07-05 Thread Sascha Hauer
Measurements on i.MX1 and i.MX53 have shown that the divider values
in the datasheets are wrong. the values from first, third and fourth
column were all measured to be 8 higher than in the datasheet. It
should be safe to assume that the SoCs between i.MX1 and i.MX53 behave
the same as the i2c unit is unchanged since the i.MX1.

Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
---
 drivers/i2c/busses/i2c-imx.c |   31 ++-
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 8d6b504..aabbe31 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -98,22 +98,27 @@
  * Document Number: MC9328MXLRM, Rev. 5.1, 06/2007
  *
  * Duplicated divider values removed from list
+ *
+ * These values (mostly) do not match the ones in the datasheet because
+ * measurements have shown that the values are wrong. This was tested
+ * on i.MX1 and i.MX53, so it shoud be safe to assume that the SoCs in
+ * between behave the same.
  */
 
 static u16 __initdata i2c_clk_div[50][2] = {
-   { 22,   0x20 }, { 24,   0x21 }, { 26,   0x22 }, { 28,   0x23 },
-   { 30,   0x00 }, { 32,   0x24 }, { 36,   0x25 }, { 40,   0x26 },
-   { 42,   0x03 }, { 44,   0x27 }, { 48,   0x28 }, { 52,   0x05 },
-   { 56,   0x29 }, { 60,   0x06 }, { 64,   0x2A }, { 72,   0x2B },
-   { 80,   0x2C }, { 88,   0x09 }, { 96,   0x2D }, { 104,  0x0A },
-   { 112,  0x2E }, { 128,  0x2F }, { 144,  0x0C }, { 160,  0x30 },
-   { 192,  0x31 }, { 224,  0x32 }, { 240,  0x0F }, { 256,  0x33 },
-   { 288,  0x10 }, { 320,  0x34 }, { 384,  0x35 }, { 448,  0x36 },
-   { 480,  0x13 }, { 512,  0x37 }, { 576,  0x14 }, { 640,  0x38 },
-   { 768,  0x39 }, { 896,  0x3A }, { 960,  0x17 }, { 1024, 0x3B },
-   { 1152, 0x18 }, { 1280, 0x3C }, { 1536, 0x3D }, { 1792, 0x3E },
-   { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D },
-   { 3072, 0x1E }, { 3840, 0x1F }
+   { 30,   0x20 }, { 32,   0x21 }, { 34,   0x22 }, { 36,   0x23 },
+   { 38,   0x00 }, { 40,   0x24 }, { 44,   0x25 }, { 48,   0x26 },
+   { 50,   0x03 }, { 52,   0x27 }, { 56,   0x28 }, { 60,   0x05 },
+   { 64,   0x29 }, { 68,   0x06 }, { 72,   0x2A }, { 80,   0x2B },
+   { 88,   0x2C }, { 96,   0x09 }, { 104,  0x2D }, { 112,  0x0A },
+   { 120,  0x2E }, { 136,  0x2F }, { 152,  0x0C }, { 168,  0x30 },
+   { 200,  0x31 }, { 232,  0x32 }, { 248,  0x0F }, { 264,  0x33 },
+   { 288,  0x10 }, { 328,  0x34 }, { 392,  0x35 }, { 456,  0x36 },
+   { 480,  0x13 }, { 520,  0x37 }, { 576,  0x14 }, { 648,  0x38 },
+   { 776,  0x39 }, { 904,  0x3A }, { 960,  0x17 }, { 1032, 0x3B },
+   { 1152, 0x18 }, { 1288, 0x3C }, { 1544, 0x3D }, { 1800, 0x3E },
+   { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D },
+   { 3072, 0x1E }, { 3840, 0x1F }
 };
 
 struct imx_i2c_struct {
-- 
1.7.10

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Thomas Petazzoni
Le Thu, 5 Jul 2012 15:15:22 +0200,
Andrew Lunn and...@lunn.ch a écrit :

   The biggest problem i had, is the interaction between generic chip
   interrupts and irqdomain. There has been work to integrate the two,
   but its stalled. Either the work needs restarting and completing, or
   you need to throw away the use of generic interrupt so that you can
   use irqdomain linear. IMHO, throwing away generic interrupt is the
   wrong way.
  
  Can you expand on why you think it would be wrong to throw away the
  usage of irq_chip_generic, compared to implementing directly an
  irq_chip?
 
 Basically you are asking, why should i use the framework when i can do
 it by hand.

Agreed :)

 What are the advantages if ignoring the framework and doing it by
 hand?

Many GPIO drivers directly use irq_chip (though it's true some of them
use irq_chip_generic), and the irq_chip_generic framework doesn't have
really proper support for the other new irqdomain framework, so I was
wondering what the best solution was.

I found the proposal from Rob Herring to improve that situation, but
apparently the conclusion from Grant Likely was Let's discuss this at
the next Connect. That patch set was posted in January, so two Linaro
Connects happened since them, I don't know if progress has been made
here.

I guess I'll just stick to irq_chip_generic + irqdomain_legacy for now.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Arnd Bergmann
On Thursday 05 July 2012, Andrew Lunn wrote:

  
  I'm wondering about this one.  The other platforms usually put the secondary
  interrupt controllers into the same match table, while you call 
  orion_gpio_of_init
  from orion_add_irq_domain. Can you explain why you do this? Does it have
  any disadvantages?
 
 The issue is knowing what IRQ number to use for the secondary
 interrupts.
 
 Orion use generic chip interrupts, both for the main interrupts and
 the GPIO interrupts. This does not yet support irq domain, so i have
 to layer a legacy domain on top. The legacy domain needs to know the
 first IRQ and the number of IRQs. For the primary IRQs that is
 easy. However, GPIO IRQ is not so easy, it depends on how many primary
 IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood,
 64, and mv78xx0 has 96. I need to know this number when adding the
 GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in
 the orion_add_irq_domain() i have this number to hand. If i used to
 entries in the match table, i would have to put this number into some
 global variable, or somehow ask the IRQ subsystem what the next free
 IRQ number is.

But couldn't you store the number of interrupts for the primary controller
in a local variable? I think the of_irq code already guarantees that
the parent is probed first.

 As for disadvantages, humm. Dove has yet more interrupts, from the
 PMU. They are currently unsupported in DT. When we add support for the
 PMU interrupt controller, we are going to have the same problem, what
 IRQ base should it use. Either we extend the chaining, calling
 dove_pmu_of_init from orion_gpio_of_init(), where we know the next
 free IRQ. Or we find out how to ask the IRQ subsystem for the next
 available. Better still, the work to make generic chip interrupts irq
 domain aware would get completed, and we can swap all this code to irq
 domain linear and this whole probable probably goes away.

Yes, that makes sense. Using the linear domain should solve all these
nicely.

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Sebastian Hesselbarth

On 07/05/2012 03:08 PM, Andrew Lunn wrote:

The issue is knowing what IRQ number to use for the secondary
interrupts.

Orion use generic chip interrupts, both for the main interrupts and
the GPIO interrupts. This does not yet support irq domain, so i have
to layer a legacy domain on top. The legacy domain needs to know the
first IRQ and the number of IRQs. For the primary IRQs that is
easy. However, GPIO IRQ is not so easy, it depends on how many primary
IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood,
64, and mv78xx0 has 96. I need to know this number when adding the
GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in
the orion_add_irq_domain() i have this number to hand. If i used to
entries in the match table, i would have to put this number into some
global variable, or somehow ask the IRQ subsystem what the next free
IRQ number is.


Andrew,

is it possible to group all gpio banks into one DT description?
For mach-dove it could be something like:

gpio: gpio-controller {
compatible = marvell, orion-gpio;
...

bank0@d0400 {
reg = 0xd0400 0x40;
ngpio = 8;
mask-offset = 0;
interrupts = 12;
};

bank1@d0400 {
reg = 0xd0400 0x40;
ngpio = 8;
mask-offset = 8;
interrupts = 13;
};

...

bank4@d0420 {
reg = 0xd0420 0x40;
ngpio = 32;
interrupts = 61;
};  

bank5@e8400 {
reg = 0xe8400 0x20;
ngpio = 8;
marvell,orion-gpio-output-only;
};  
};

This would have the advantage that DT describes gpio-to-irq dependencies.
Moreover, nodes that reference gpios can do gpios = gpio 71 0; instead of
gpios = gpio3 7 0;

Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Andrew Lunn
On Thu, Jul 05, 2012 at 04:14:50PM +0200, Sebastian Hesselbarth wrote:
 On 07/05/2012 03:08 PM, Andrew Lunn wrote:
 The issue is knowing what IRQ number to use for the secondary
 interrupts.
 
 Orion use generic chip interrupts, both for the main interrupts and
 the GPIO interrupts. This does not yet support irq domain, so i have
 to layer a legacy domain on top. The legacy domain needs to know the
 first IRQ and the number of IRQs. For the primary IRQs that is
 easy. However, GPIO IRQ is not so easy, it depends on how many primary
 IRQs there are. This is not fixed. Orion5x has 32, Dove 64, kirkwood,
 64, and mv78xx0 has 96. I need to know this number when adding the
 GPIO secondary IRQ legacy domain. By calling orion_gpio_of_init() in
 the orion_add_irq_domain() i have this number to hand. If i used to
 entries in the match table, i would have to put this number into some
 global variable, or somehow ask the IRQ subsystem what the next free
 IRQ number is.
 
 Andrew,
 
 is it possible to group all gpio banks into one DT description?

Everything is possible. 

I did think about having just one gpio controller, since as you said,
it makes it easier to describe gpios = gpio 71 0; instead of gpios
= gpio3 7 0; But most SoCs seem to have multiple GPIO controllers
in the .dtsi files. Only picoxcell has banks inside a single gpio
controller. So it would be a bit unusual.
 
What you suggest also adds a lot of complexity to the code and
probably means a lot less of the code can be shared with non-DT GPIO
handling code.

The natural size of a GPIO controller is 32 lines, not 8. The current
interrupt handling does not actually care which IRQ of a bank of 32
lines fired, it looks at all lines. The handler_data in the IRQ points
to the GPIO chip, not a subsection of the GPIO chip.

   bank5@e8400 {
   reg = 0xe8400 0x20;
   ngpio = 8;
   marvell,orion-gpio-output-only;

Some Orions mixed GPIOs GPOs and GPIs within one controller. So that
is not generic enough. Probably the pinmux DT description needs to be
able to specify per pin what a line can do.

 Andrew
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 8/8 v3] i2c: i801: enable irq for byte_by_byte transactions

2012-07-05 Thread Jean Delvare
Hi Daniel,

On Wed, 27 Jun 2012 21:54:15 +0800, Daniel Kurtz wrote:
 Byte-by-byte transactions are used primarily for accessing I2C devices
 with an SMBus controller.  For these transactions, for each byte that is
 read or written, the SMBus controller generates a BYTE_DONE irq.  The isr
 reads/writes the next byte, and clears the irq flag to start the next byte.
 On the penultimate irq, the isr also sets the LAST_BYTE flag.
 
 There is no locking around the cmd/len/count/data variables, since the
 I2C adapter lock ensures there is never multiple simultaneous transactions
 for the same device, and the driver thread never accesses these variables
 while interrupts might be occurring.
 
 The end result is faster I2C block read and write transactions.
 
 Note: This patch has only been tested and verified by doing I2C read and
 write block transfers on Cougar Point 6 Series PCH.

Two issues remaining:

 +static void i801_isr_byte_done(struct i801_priv *priv)
 +{
 + /* For SMBus block reads, length is first byte read */
 + if (priv-is_read  ((priv-cmd  0x1c) == I801_BLOCK_DATA) 
 + (priv-count == 0)) {
 + priv-len = inb_p(SMBHSTDAT0(priv));
 + if (priv-len  1 || priv-len  I2C_SMBUS_BLOCK_MAX) {
 + dev_err(priv-pci_dev-dev,
 + Illegal SMBus block read size %d\n,
 + priv-len);
 + /* FIXME: Recover */
 + priv-len = I2C_SMBUS_BLOCK_MAX;
 + } else {
 + dev_dbg(priv-pci_dev-dev,
 + SMBus block read size is %d\n,
 + priv-len);
 + }
 + priv-data[-1] = priv-len;
 + } else if (priv-is_read) {

The else is wrong. When count == 0, you receive two bytes from the
controller: the block length in SMBHSTDAT0 and the first data byte in
SMBBLKDAT. So the code flow must fall through.

 + priv-data[priv-count++] = inb(SMBBLKDAT(priv));

This is lacking a boundary check. As I reported in an earlier review,
you shouldn't assume that the hardware will only emit the number of
interrupts you are expecting. If for any reason (crazy hardware or
driver bug) you get more interrupts than expected, you don't want to
overflow priv-data[].

 + /* Set LAST_BYTE for last byte of read transaction */
 + if (priv-count == priv-len - 1)
 + priv-cmd |= SMBHSTCNT_LAST_BYTE;
 + outb_p(priv-cmd, SMBHSTCNT(priv));
 + } else if (priv-count  priv-len - 1) {
 + /* Write next byte, except for IRQ after last byte */
 + outb_p(priv-data[++priv-count], SMBBLKDAT(priv));
 + outb_p(priv-cmd, SMBHSTCNT(priv));
 + }
 +
 + /* Clear BYTE_DONE to start next transaction. */
 + outb_p(SMBHSTSTS_BYTE_DONE, SMBHSTSTS(priv));
 +}

The rest looks OK.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c i.MX: Fix divider table

2012-07-05 Thread Richard Zhao
On Thu, Jul 05, 2012 at 03:10:26PM +0200, Sascha Hauer wrote:
 Measurements on i.MX1 and i.MX53 have shown that the divider values
 in the datasheets are wrong. the values from first, third and fourth
 column were all measured to be 8 higher than in the datasheet. It
 should be safe to assume that the SoCs between i.MX1 and i.MX53 behave
 the same as the i2c unit is unchanged since the i.MX1.
We may need to check with IC guys? Or you've done?

Thanks
Richard
 
 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
 ---
  drivers/i2c/busses/i2c-imx.c |   31 ++-
  1 file changed, 18 insertions(+), 13 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
 index 8d6b504..aabbe31 100644
 --- a/drivers/i2c/busses/i2c-imx.c
 +++ b/drivers/i2c/busses/i2c-imx.c
 @@ -98,22 +98,27 @@
   * Document Number: MC9328MXLRM, Rev. 5.1, 06/2007
   *
   * Duplicated divider values removed from list
 + *
 + * These values (mostly) do not match the ones in the datasheet because
 + * measurements have shown that the values are wrong. This was tested
 + * on i.MX1 and i.MX53, so it shoud be safe to assume that the SoCs in
 + * between behave the same.
   */
  
  static u16 __initdata i2c_clk_div[50][2] = {
 - { 22,   0x20 }, { 24,   0x21 }, { 26,   0x22 }, { 28,   0x23 },
 - { 30,   0x00 }, { 32,   0x24 }, { 36,   0x25 }, { 40,   0x26 },
 - { 42,   0x03 }, { 44,   0x27 }, { 48,   0x28 }, { 52,   0x05 },
 - { 56,   0x29 }, { 60,   0x06 }, { 64,   0x2A }, { 72,   0x2B },
 - { 80,   0x2C }, { 88,   0x09 }, { 96,   0x2D }, { 104,  0x0A },
 - { 112,  0x2E }, { 128,  0x2F }, { 144,  0x0C }, { 160,  0x30 },
 - { 192,  0x31 }, { 224,  0x32 }, { 240,  0x0F }, { 256,  0x33 },
 - { 288,  0x10 }, { 320,  0x34 }, { 384,  0x35 }, { 448,  0x36 },
 - { 480,  0x13 }, { 512,  0x37 }, { 576,  0x14 }, { 640,  0x38 },
 - { 768,  0x39 }, { 896,  0x3A }, { 960,  0x17 }, { 1024, 0x3B },
 - { 1152, 0x18 }, { 1280, 0x3C }, { 1536, 0x3D }, { 1792, 0x3E },
 - { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D },
 - { 3072, 0x1E }, { 3840, 0x1F }
 + { 30,   0x20 }, { 32,   0x21 }, { 34,   0x22 }, { 36,   0x23 },
 + { 38,   0x00 }, { 40,   0x24 }, { 44,   0x25 }, { 48,   0x26 },
 + { 50,   0x03 }, { 52,   0x27 }, { 56,   0x28 }, { 60,   0x05 },
 + { 64,   0x29 }, { 68,   0x06 }, { 72,   0x2A }, { 80,   0x2B },
 + { 88,   0x2C }, { 96,   0x09 }, { 104,  0x2D }, { 112,  0x0A },
 + { 120,  0x2E }, { 136,  0x2F }, { 152,  0x0C }, { 168,  0x30 },
 + { 200,  0x31 }, { 232,  0x32 }, { 248,  0x0F }, { 264,  0x33 },
 + { 288,  0x10 }, { 328,  0x34 }, { 392,  0x35 }, { 456,  0x36 },
 + { 480,  0x13 }, { 520,  0x37 }, { 576,  0x14 }, { 648,  0x38 },
 + { 776,  0x39 }, { 904,  0x3A }, { 960,  0x17 }, { 1032, 0x3B },
 + { 1152, 0x18 }, { 1288, 0x3C }, { 1544, 0x3D }, { 1800, 0x3E },
 + { 1920, 0x1B }, { 2048, 0x3F }, { 2304, 0x1C }, { 2560, 0x1D },
 + { 3072, 0x1E }, { 3840, 0x1F }
  };
  
  struct imx_i2c_struct {
 -- 
 1.7.10
 
 
 ___
 linux-arm-kernel mailing list
 linux-arm-ker...@lists.infradead.org
 http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Arnd Bergmann
On Thursday 05 July 2012, Sebastian Hesselbarth wrote:
 Andrew,
 
 is it possible to group all gpio banks into one DT description?
 For mach-dove it could be something like:
 
 gpio: gpio-controller {
 compatible = marvell, orion-gpio;
 ...
 
 bank0@d0400 {
 reg = 0xd0400 0x40;
 ngpio = 8;
 mask-offset = 0;
 interrupts = 12;
 };
 
 bank1@d0400 {
 reg = 0xd0400 0x40;
 ngpio = 8;
 mask-offset = 8;
 interrupts = 13;
 };

This way you have multiple nodes with the same register
and different names, which is not how it normally works.

 
 This would have the advantage that DT describes gpio-to-irq dependencies.
 Moreover, nodes that reference gpios can do gpios = gpio 71 0; instead of
 gpios = gpio3 7 0;

Is that desired?

The device tree representation should match what is in the data sheet
normally. If they are in a single continuous number range, then we should
probably have a single device node with multiple register ranges
rather than one device node for each 32-bit register. Looking at
arch/arm/plat-orion/gpio.c I think that is not actually the case though
and having separate banks is more logical.

Something completely different I just noticed in the original patch:

 @@ -90,6 +74,27 @@ static void pmu_irq_handler(unsigned int irq, struct 
 irq_desc *desc)
   }
  }
  
 +static int __initdata gpio0_irqs[4] = {
 + IRQ_DOVE_GPIO_0_7,
 + IRQ_DOVE_GPIO_8_15,
 + IRQ_DOVE_GPIO_16_23,
 + IRQ_DOVE_GPIO_24_31,
 +};
 +
 +static int __initdata gpio1_irqs[4] = {
 + IRQ_DOVE_HIGH_GPIO,
 + 0,
 + 0,
 + 0,
 +};

I think the latter one needs to be

+static int __initdata gpio1_irqs[4] = {
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+};

so we register all four parts to the same primary IRQ. The
same is true for the devicetree representation.

Arnd

--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Sebastian Hesselbarth

On 07/05/2012 04:54 PM, Arnd Bergmann wrote:

This way you have multiple nodes with the same register
and different names, which is not how it normally works.


Ok.


This would have the advantage that DT describes gpio-to-irq dependencies.
Moreover, nodes that reference gpios can do gpios =gpio 71 0; instead of
gpios =gpio3 7 0;


Is that desired?

The device tree representation should match what is in the data sheet
normally. If they are in a single continuous number range, then we should
probably have a single device node with multiple register ranges
rather than one device node for each 32-bit register. Looking at
arch/arm/plat-orion/gpio.c I think that is not actually the case though
and having separate banks is more logical.


Well, looking at the datasheet of Dove GPIOs are numbered [63:0] plus
GPOs [71:64]. This dt will be a lot shorter and maybe it is describing
the hardware as it is. (Not sure about the syntax for irqs, though)

gpio@d0400 {
compatible = marvell,orion-gpio;
gpio-controller;
reg = 0xd0400 0x20, /* GPIO[31: 0] */
  0xd0420 0x20, /* GPIO[63:32] */
  0xe8400 0x0c; /* GPO [71:64] */
ngpio = 72;
interrupts = 12 13 14 15, 61;
};

Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Rtp
Sebastian Hesselbarth sebastian.hesselba...@googlemail.com writes:

 On 07/05/2012 04:54 PM, Arnd Bergmann wrote:
 This way you have multiple nodes with the same register
 and different names, which is not how it normally works.

 Ok.

 This would have the advantage that DT describes gpio-to-irq dependencies.
 Moreover, nodes that reference gpios can do gpios =gpio 71 0; instead of
 gpios =gpio3 7 0;

 Is that desired?

 The device tree representation should match what is in the data sheet
 normally. If they are in a single continuous number range, then we should
 probably have a single device node with multiple register ranges
 rather than one device node for each 32-bit register. Looking at
 arch/arm/plat-orion/gpio.c I think that is not actually the case though
 and having separate banks is more logical.

 Well, looking at the datasheet of Dove GPIOs are numbered [63:0] plus
 GPOs [71:64]. This dt will be a lot shorter and maybe it is describing
 the hardware as it is. (Not sure about the syntax for irqs, though)

They're numbered as [63:0] and [71:64] but they're on 3 different banks.
iirc, there may even be some differences with the way the banks are dealing
interrupts, so I don't see any reason to not represent the 3 banks in DT.

Arnaud
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c i.MX: Fix divider table

2012-07-05 Thread Shubhrajyoti Datta
On Thu, Jul 5, 2012 at 6:40 PM, Sascha Hauer s.ha...@pengutronix.de wrote:
 Measurements on i.MX1 and i.MX53 have shown that the divider values
 in the datasheets are wrong.
How were the measurements made?

 the values from first, third and fourth
 column were all measured to be 8 higher than in the datasheet. It
 should be safe to assume that the SoCs between i.MX1 and i.MX53 behave
 the same as the i2c unit is unchanged since the i.MX1.

Also does it vary board to board or is fixed by the ip?
What I mean is that the external cap etc.




 Signed-off-by: Sascha Hauer s.ha...@pengutronix.de
 ---
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 01/12] ARM: Orion: DT support for IRQ and GPIO Controllers

2012-07-05 Thread Mitch Bradley

On 7/5/2012 4:54 AM, Arnd Bergmann wrote:

On Thursday 05 July 2012, Sebastian Hesselbarth wrote:

Andrew,

is it possible to group all gpio banks into one DT description?
For mach-dove it could be something like:

gpio: gpio-controller {
 compatible = marvell, orion-gpio;
 ...

 bank0@d0400 {
 reg = 0xd0400 0x40;
 ngpio = 8;
 mask-offset = 0;
 interrupts = 12;
 };

 bank1@d0400 {
 reg = 0xd0400 0x40;
 ngpio = 8;
 mask-offset = 8;
 interrupts = 13;
 };


This way you have multiple nodes with the same register
and different names, which is not how it normally works.


The mask-offset property is really a reg in disguise.

reg is considerably more general than just memory mapped
register address.  It really means any numeric identifier
that makes sense in the context of a parent device.

Therefore, one could say:

gpio: gpio-controller {
 compatible = marvell, orion-gpio;
 reg = 0xd0400 0x40;
 #address-cells = 1;
 #size-cells = 0;
 ...

 bank0@0 {
 reg = 0x0;
 ngpio = 8;
 mask-offset = 0;
 interrupts = 12;
 };

 bank1@8 {
 reg = 0x8;
 ngpio = 8;
 interrupts = 13;
  };





This would have the advantage that DT describes gpio-to-irq dependencies.
Moreover, nodes that reference gpios can do gpios = gpio 71 0; instead of
gpios = gpio3 7 0;


Is that desired?

The device tree representation should match what is in the data sheet
normally. If they are in a single continuous number range, then we should
probably have a single device node with multiple register ranges
rather than one device node for each 32-bit register. Looking at
arch/arm/plat-orion/gpio.c I think that is not actually the case though
and having separate banks is more logical.

Something completely different I just noticed in the original patch:


@@ -90,6 +74,27 @@ static void pmu_irq_handler(unsigned int irq, struct 
irq_desc *desc)
   }
  }

+static int __initdata gpio0_irqs[4] = {
+ IRQ_DOVE_GPIO_0_7,
+ IRQ_DOVE_GPIO_8_15,
+ IRQ_DOVE_GPIO_16_23,
+ IRQ_DOVE_GPIO_24_31,
+};
+
+static int __initdata gpio1_irqs[4] = {
+ IRQ_DOVE_HIGH_GPIO,
+ 0,
+ 0,
+ 0,
+};


I think the latter one needs to be

+static int __initdata gpio1_irqs[4] = {
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+ IRQ_DOVE_HIGH_GPIO,
+};

so we register all four parts to the same primary IRQ. The
same is true for the devicetree representation.

Arnd

___
devicetree-discuss mailing list
devicetree-disc...@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss



--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] i2c i.MX: Fix divider table

2012-07-05 Thread Sascha Hauer
On Thu, Jul 05, 2012 at 11:36:27PM +0530, Shubhrajyoti Datta wrote:
 On Thu, Jul 5, 2012 at 6:40 PM, Sascha Hauer s.ha...@pengutronix.de wrote:
  Measurements on i.MX1 and i.MX53 have shown that the divider values
  in the datasheets are wrong.
 How were the measurements made?

With an oscilloscope measuring the period length. This is probably not
very exact, but the current values are way off. We had 350KHz instead
of 380KHz (best possible divider) on an i.MX53. Funny enough some
divider values really match the ones in the datasheet.

BTW I'm pretty sure the input frequency to the core is calculated
correctly as it's the same clock that also drives the timer.

 
  the values from first, third and fourth
  column were all measured to be 8 higher than in the datasheet. It
  should be safe to assume that the SoCs between i.MX1 and i.MX53 behave
  the same as the i2c unit is unchanged since the i.MX1.
 
 Also does it vary board to board or is fixed by the ip?
 What I mean is that the external cap etc.

The clock is derived from an internal SoC clock, I don't think this can
be influenced that much by external components. I did not test different
boards, but two board with different SoCs.

Let's see what the IC guys tell us.

Sascha

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Hid over I2C and ACPI interaction

2012-07-05 Thread Mika Westerberg
On Thu, Jul 05, 2012 at 03:01:57PM +0800, Zhang Rui wrote:
 +Note that although these are ACPI devices, we prefer to use PnP drivers
 for them,
 +this is because:
 +1. all the non-ACPI-predefined Devices are exported as PnP devices as
 well
 +2. PnP bus is a well designed bus. Probing via PnP layer saves a lot of
 work
 +   for the device driver, e.g. getting  parsing ACPI resources.

(Nice BKM, thanks for sharing)

I have few questions about using PnP drivers instead of pure ACPI drivers.

ACPI 5.0 defined some new resources, for example Fixed DMA descriptor
that has information about the request line + channel for the device to
use. Hovewer, PnP drivers pass resources as 'struct resource', which
basically only has start and end - how do you represent all this new stuff
using 'struct resource'?

Or should we use acpi_walk_resources() where 'struct resource' is not
suitable?
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html