Re: Fwd: Hid over I2C and ACPI interaction
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
- 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
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
+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
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
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
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
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
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
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
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
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
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
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
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
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
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
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