I2C adapters protocol deviation

Hi Wolfram,

On the Allwinner A31, the PMIC communicates with the SoC through a bus looking quite similar to I2C, while being pretty different.

The communication starts with the PMIC through the regular I2C protocol, but it's only used to initialize the PMIC, and switch to a mode called "Push/Pull 2 Wire Interface".

That bus is using SDA and SCL, with the start and stop conditions exactly like I2C does, but:
- Once the start condition has been issued, the address isn't sent, only a direction bit. Hence, it does not support multiple devices anymore.
- Once that direction bit has been sent, the master sends the register it wants to read from/write to, over 8 bits, followed by one parity bit.
- Whenever you're writing, the master then sends the data over 8 bits, followed once again by a parity bit. Then, and only then, an ACK is issued by the slave.
- Whenever you're reading, the master then clocks SDL and the slave drives SDA for 8 bits plus 1 parity bit. If there was some kind of error, the slave will pull SDA up for 9 cycles, resulting in a parity error. Like with I2C though, since it is the only and last byte it's receiving, the master won't issue an ACK.

Obviously, to go ahead with the PMIC support, we need to support this controller and bus first. I can't really decide whether it's within the scope of the allowed protocol deviations of I2C or if we should create a whole new bus for it.

Hi,

Quoted 26 lines · Maxime Ripard

Hi Wolfram,

On the Allwinner A31, the PMIC communicates with the SoC through a bus looking quite similar to I2C, while being pretty different.

The communication starts with the PMIC through the regular I2C protocol, but it's only used to initialize the PMIC, and switch to a mode called "Push/Pull 2 Wire Interface".

That bus is using SDA and SCL, with the start and stop conditions exactly like I2C does, but:
- Once the start condition has been issued, the address isn't sent, only a direction bit. Hence, it does not support multiple devices anymore.
- Once that direction bit has been sent, the master sends the register it wants to read from/write to, over 8 bits, followed by one parity bit.
- Whenever you're writing, the master then sends the data over 8 bits, followed once again by a parity bit. Then, and only then, an ACK is issued by the slave.
- Whenever you're reading, the master then clocks SDL and the slave drives SDA for 8 bits plus 1 parity bit. If there was some kind of error, the slave will pull SDA up for 9 cycles, resulting in a parity error. Like with I2C though, since it is the only and last byte it's receiving, the master won't issue an ACK.

Quoted 4 lines

Obviously, to go ahead with the PMIC support, we need to support this controller and bus first. I can't really decide whether it's within the scope of the allowed protocol deviations of I2C or if we should create a whole new bus for it.

I've been thinking about this too. In the mean time I have slept quite a few nights on this now and swung my opinion from use i2c subsys to do our own bus and back again. But since I've actually added support for this to u-boot and I now know the device better I believe using the i2c subsys is the best solution for this.

The PMIC does seem to start in i2c, and then gets switched to this new push-pull 2 wire i2c-ish protocol on init. So it does have a slave address, but that is used only once, and then the bus is for a single device only.

Note there seems to be no way to use the host in a regular i2c mode. It has a setup mode where it does a single regular i2c write (or so I believe) and then it is in its own custom mode from there on.

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

IOW the i2c_algorithm struct for the host would not set master_xfer, only smbus_xfer and its functionality function would return I2C_FUNC_SMBUS_BYTE

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

Quoted 9 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

Are there some datasheets available?

Hi,

Quoted 10 lines · Wolfram Sang

Quoted 9 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

Quoted 7 lines

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Thanks for the input this sounds good, I guess we'll give this a shot when we get around to coding up support for the p2wi block in the A31.

Quoted 2 lines

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

The datasheet we've suggests that it actually influences the one slave attached. Note that u-boot on this machines will likely already have made the switch, but I guess we don't want to count on that.

Quoted 1 line

Are there some datasheets available?

The AXP221 is documented here:
http://linux-sunxi.org/AXP221
This is translated by one of our community members from a Chinese datasheet.

The P2WI interface is (somewhat) documented in the A31 datasheet, but we cannot share that in public.

Quoted 21 lines · Hans de Goede

Hi,

Quoted 17 lines · Wolfram Sang

Quoted 9 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Quoted 2 lines

Thanks for the input this sounds good, I guess we'll give this a shot when we get around to coding up support for the p2wi block in the A31.

On a second thought, maybe more granularity is better. Like using I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY.

Quoted 6 lines

Quoted 2 lines

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

The datasheet we've suggests that it actually influences the one slave attached. Note that u-boot on this machines will likely already have made the switch, but I guess we don't want to count on that.

Can we detect if this switching was already made?

Hi,

Quoted 21 lines · Wolfram Sang

Quoted 20 lines · Hans de Goede

Hi,

Quoted 8 lines · Wolfram Sang

Quoted 8 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

Quoted 7 lines

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Thanks for the input this sounds good, I guess we'll give this a shot when we get around to coding up support for the p2wi block in the A31.

Quoted 3 lines

On a second thought, maybe more granularity is better. Like using I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY.

Hmm, I'm not completely sold on the whole idea of having special flags, esp. since it seems that ie the AXP221 may operate in normal i2c mode in some designs too. So ideally we would just hide from clients that this is something else then plain i2c. So that we can have an axp221 driver which is not even aware about this weird i2c-variant and will just work independent on how the axp221 is hooked up.

Likewise it would be useful to have the i2cdump utility just work, etc.

So maybe a flag which is a hint that this is special on the controller, but I don't think we should be checking for special flags in the messages on the controller side. Basically the whole p2wi allows reading / writing byte registers, so I2C_FUNC_SMBUS_BYTE is a 1:1 mapping of the functionality, as for the address, we can just check it is the one address used to do the initial setup, and if it is not then just return an error.

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

The datasheet we've suggests that it actually influences the one slave attached. Note that u-boot on this machines will likely already have made the switch, but I guess we don't want to count on that.

Can we detect if this switching was already made?

I don't think we can. But I think doing the switch a second time is ok / does not result in an error.

Regards,

Hans

Hi,

Quoted 22 lines · Hans de Goede

Hi,

Quoted 19 lines · Wolfram Sang

Quoted 18 lines · Hans de Goede

Hi,

Quoted 15 lines · Wolfram Sang

Quoted 8 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Thanks for the input this sounds good, I guess we'll give this a shot when we get around to coding up support for the p2wi block in the A31.

Hans, I started to work on the p2wi driver, but let me know if you already have a working driver (I'll either rebase my work on yours or let you finish your implementation).

Quoted 4 lines

Quoted 4 lines

On a second thought, maybe more granularity is better. Like using I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY.

For the moment I only make use of the I2C_CLIENT_PUSHPULL and I2C_M_PUSHPULL flags, but I can add those flags (and check them in the smbus_xfer callback) if you think this is relevant.

Quoted 6 lines

Hmm, I'm not completely sold on the whole idea of having special flags, esp. since it seems that ie the AXP221 may operate in normal i2c mode in some designs too. So ideally we would just hide from clients that this is something else then plain i2c. So that we can have an axp221 driver which is not even aware about this weird i2c-variant and will just work independent on how the axp221 is hooked up.

Hans, can't we use the same PMIC driver but with different i2c flags for the p2wi case.

IMHO, the real question here is: do we want to support the p2wi protocol on other i2c bus implementations (like the i2c bitbanging driver) ? If this is the case we definitely need those specific flags.

Quoted 15 lines

Likewise it would be useful to have the i2cdump utility just work, etc.

So maybe a flag which is a hint that this is special on the controller, but I don't think we should be checking for special flags in the messages on the controller side. Basically the whole p2wi allows reading / writing byte registers, so I2C_FUNC_SMBUS_BYTE is a 1:1 mapping of the functionality, as for the address, we can just check it is the one address used to do the initial setup, and if it is not then just return an error.

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

The datasheet we've suggests that it actually influences the one slave attached. Note that u-boot on this machines will likely already have made the switch, but I guess we don't want to count on that.

Can we detect if this switching was already made?

Quoted 2 lines

I don't think we can. But I think doing the switch a second time is ok / does not result in an error.

AFAICT you cannot detect if the i2c to p2wi switch has already taken place, and re launching the switch process might have unexpected effects:
let's say the i2c address you're trying to access maps to a valid PMIC register, as the address should be dropped in p2wi mode, you might write an erroneous value to this register.

Hi,

Quoted 22 lines · Wolfram Sang

Quoted 21 lines · Hans de Goede

Hi,

Quoted 17 lines · Wolfram Sang

Quoted 9 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Quoted 6 lines

Quoted 2 lines

Thanks for the input this sounds good, I guess we'll give this a shot when we get around to coding up support for the p2wi block in the A31.

On a second thought, maybe more granularity is better. Like using I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY.

Hmm, I'm not completely sold on the whole idea of having special flags, esp. since it seems that ie the AXP221 may operate in normal i2c mode in some designs too. So ideally we would just hide from clients that this is something else then plain i2c. So that we can have an axp221 driver which is not even aware about this weird i2c-variant and will just work independent on how the axp221 is hooked up.

Likewise it would be useful to have the i2cdump utility just work, etc.

So maybe a flag which is a hint that this is special on the controller, but I don't think we should be checking for special flags in the messages on the controller side. Basically the whole p2wi allows reading / writing byte registers, so I2C_FUNC_SMBUS_BYTE is a 1:1 mapping of the functionality, as for the address, we can just check it is the one address used to do the initial setup, and if it is not then just return an error.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

The datasheet we've suggests that it actually influences the one slave attached. Note that u-boot on this machines will likely already have made the switch, but I guess we don't want to count on that.

Can we detect if this switching was already made?

I don't think we can. But I think doing the switch a second time is ok / does not result in an error.

Quoted 32 lines · Hans de Goede

Hi,

Quoted 22 lines · Wolfram Sang

Quoted 21 lines · Hans de Goede

Hi,

Quoted 17 lines · Wolfram Sang

Quoted 9 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Quoted 6 lines

Quoted 2 lines

Thanks for the input this sounds good, I guess we'll give this a shot when we get around to coding up support for the p2wi block in the A31.

On a second thought, maybe more granularity is better. Like using I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY.

Quoted 6 lines

Hmm, I'm not completely sold on the whole idea of having special flags, esp. since it seems that ie the AXP221 may operate in normal i2c mode in some designs too. So ideally we would just hide from clients that this is something else then plain i2c. So that we can have an axp221 driver which is not even aware about this weird i2c-variant and will just work independent on how the axp221 is hooked up.

I don't think we actually saw in real life an AXP221 connected only using i2c. I'd say we shouldn't worry too much about a theorical corner case that we never saw, until we actually see it.

Quoted 1 line

Likewise it would be useful to have the i2cdump utility just work, etc.

I'm not sure I want the i2c-tools to start poking around the PMIC.

Quoted 7 lines

So maybe a flag which is a hint that this is special on the controller, but I don't think we should be checking for special flags in the messages on the controller side. Basically the whole p2wi allows reading / writing byte registers, so I2C_FUNC_SMBUS_BYTE is a 1:1 mapping of the functionality, as for the address, we can just check it is the one address used to do the initial setup, and if it is not then just return an error.

Yes, we obviously have to check for the address in the xfer function.

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Quoted 2 lines

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

The datasheet we've suggests that it actually influences the one slave attached. Note that u-boot on this machines will likely already have made the switch, but I guess we don't want to count on that.

Quoted 4 lines

Quoted 1 line

Can we detect if this switching was already made?

I don't think we can. But I think doing the switch a second time is ok / does not result in an error.

And it will probably mangle the PMIC configuration. I'm not very comfortable with that..

Quoted 21 lines · Hans de Goede

Hi,

Quoted 17 lines · Wolfram Sang

Quoted 9 lines

So what we really have is a single slave i2c host sort of. At least we could model it like that. The host could be told which address to listen to (and which single i2c write to do to init the pmic) through devicetree and then all the differences would be hidden in the host driver, ie we would check the slave-address and if it is not the single one we support, we just return the appropriate error for a device not acking, and everything should work as a regular i2c host which only supports i2c_smbus_read_byte and i2c_smbus_write_byte.

I'd think we need a new message flag like I2C_M_PUSHPULL which says that this message has only the direction bit instead of the address and needs a parity bit afterwards. In addition to that, we need a new functionality flag I2C_FUNC_PUSHPULL which means the host driver can handle those messages. So, the PMIC driver could query support for I2C_FUNC_SMBUS_BYTE | I2C_FUNC_PUSHPULL and if successful send messages using smbus functions with the new flag set.

Quoted 2 lines

Thanks for the input this sounds good, I guess we'll give this a shot when we get around to coding up support for the p2wi block in the A31.

On a second thought, maybe more granularity is better. Like using I2C_M_DROP_ADDRESS and I2C_M_ADD_PARITY and then make I2C_CLIENT_PUSHPULL involve I2C_M_DROP_ADDRESS | I2C_M_ADD_PARITY.

Quoted 6 lines

Quoted 2 lines

Not sure about the I2C-to-PushPull switch: Is it 100% host configuration or does it also depend on the one slave attached?

The datasheet we've suggests that it actually influences the one slave attached. Note that u-boot on this machines will likely already have made the switch, but I guess we don't want to count on that.

Can we detect if this switching was already made?

Quoted 8 lines

Quoted 1 line

Are there some datasheets available?

The AXP221 is documented here:
http://linux-sunxi.org/AXP221
This is translated by one of our community members from a Chinese datasheet.

The P2WI interface is (somewhat) documented in the A31 datasheet, but we cannot share that in public.

Any chance for me to get it if I sign something?