[PATCH 01/11] i2c: Add a DM_I2C wrapper for the sun6i_p2wi controller
Samuel Holland
samuel at sholland.org
Sun Aug 22 19:19:35 CEST 2021
Hi Heiko,
On 8/22/21 3:38 AM, Heiko Schocher wrote:
> Hello Samuel,
>
> On 22.08.21 01:05, Samuel Holland wrote:
>> This bus controller is used to communicate with an X-Powers AXP PMIC.
>> Currently, various drivers access PMIC registers through a platform-
>> specific non-DM "pmic_bus" interface, which depends on the legacy I2C
>> framework. In order to convert those drivers to use DM_PMIC, this bus
>> needs a DM_I2C driver.
>>
>> Since the non-DM bus controller driver is still needed in SPL, the quick
>> solution is to implement the DM_I2C ops using the existing functions.
>>
>> The register for switching between I2C/P2WI/RSB mode is the same across
>> all PMIC variants, so move that to the common header.
>>
>> Signed-off-by: Samuel Holland <samuel at sholland.org>
>> ---
>>
>> arch/arm/mach-sunxi/Kconfig | 11 ------
>> arch/arm/mach-sunxi/pmic_bus.c | 7 ++--
>> drivers/i2c/Kconfig | 8 +++++
>> drivers/i2c/Makefile | 1 +
>> drivers/i2c/sun6i_p2wi.c | 66 ++++++++++++++++++++++++++++++++++
>
> I wonder, as this config symbol gets also removed, that there
> is no remove of driver code?
I did not remove any config symbol, only moved it to a different file.
It sounds like you want me to use a new symbol for the DM_I2C driver
(SYS_I2C_SUN6I_P2WI). So in that case, I would leave the old symbol
alone, and have SYS_I2C_SUN6I_P2WI select it.
>> include/axp_pmic.h | 6 ++++
>> 6 files changed, 84 insertions(+), 15 deletions(-)
>> create mode 100644 drivers/i2c/sun6i_p2wi.c
>>
>> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
>> index 79c669a4813..37076c2dfb3 100644
>> --- a/arch/arm/mach-sunxi/Kconfig
>> +++ b/arch/arm/mach-sunxi/Kconfig
>> @@ -88,17 +88,6 @@ config DRAM_SUN50I_H616_UNKNOWN_FEATURE
>> feature.
>> endif
>>
>> -config SUN6I_P2WI
>> - bool "Allwinner sun6i internal P2WI controller"
>> - help
>> - If you say yes to this option, support will be included for the
>> - P2WI (Push/Pull 2 Wire Interface) controller embedded in some sunxi
>> - SOCs.
>> - The P2WI looks like an SMBus controller (which supports only byte
>> - accesses), except that it only supports one slave device.
>> - This interface is used to connect to specific PMIC devices (like the
>> - AXP221).
>> -
>> config SUN6I_PRCM
>> bool
>> help
>> diff --git a/arch/arm/mach-sunxi/pmic_bus.c b/arch/arm/mach-sunxi/pmic_bus.c
>> index 0394ce85644..673a05fdd16 100644
>> --- a/arch/arm/mach-sunxi/pmic_bus.c
>> +++ b/arch/arm/mach-sunxi/pmic_bus.c
>> @@ -8,6 +8,7 @@
>> * axp223 uses the rsb bus, these functions abstract this.
>> */
>>
>> +#include <axp_pmic.h>
>> #include <common.h>
>> #include <asm/arch/p2wi.h>
>> #include <asm/arch/rsb.h>
>> @@ -21,8 +22,6 @@
>> #define AXP305_I2C_ADDR 0x36
>>
>> #define AXP221_CHIP_ADDR 0x68
>> -#define AXP221_CTRL_ADDR 0x3e
>> -#define AXP221_INIT_DATA 0x3e
>>
>> /* AXP818 device and runtime addresses are same as AXP223 */
>> #define AXP223_DEVICE_ADDR 0x3a3
>> @@ -40,8 +39,8 @@ int pmic_bus_init(void)
>> #if defined CONFIG_AXP221_POWER || defined CONFIG_AXP809_POWER || defined CONFIG_AXP818_POWER
>> # ifdef CONFIG_MACH_SUN6I
>> p2wi_init();
>> - ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR, AXP221_CTRL_ADDR,
>> - AXP221_INIT_DATA);
>> + ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR, AXP_PMIC_MODE_REG,
>> + AXP_PMIC_MODE_P2WI);
>> # elif defined CONFIG_MACH_SUN8I_R40
>> /* Nothing. R40 uses the AXP221s in I2C mode */
>> ret = 0;
>> diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
>> index 5d27f503bfc..d082676c4b2 100644
>> --- a/drivers/i2c/Kconfig
>> +++ b/drivers/i2c/Kconfig
>> @@ -577,6 +577,14 @@ config SYS_I2C_STM32F7
>> _ Optional clock stretching
>> _ Software reset
>>
>> +config SUN6I_P2WI
>
> Could you please use "SYS_I2C_" ?
>
>> + bool "Allwinner sun6i P2WI controller"
>> + depends on ARCH_SUNXI
>> + help
>> + Support for the P2WI (Push/Pull 2 Wire Interface) controller embedded
>> + in the Allwinner A31 and A31s SOCs. This interface is used to connect
>> + to specific devices like the X-Powers AXP221 PMIC.
>> +
>> config SYNQUACER
>> bool "Socionext SynQuacer I2C controller"
>> depends on ARCH_SYNQUACER && DM_I2C
>> diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
>> index 3a7ecd9274b..2461f0a2db8 100644
>> --- a/drivers/i2c/Makefile
>> +++ b/drivers/i2c/Makefile
>> @@ -43,6 +43,7 @@ obj-$(CONFIG_SYS_I2C_SANDBOX) += sandbox_i2c.o i2c-emul-uclass.o
>> obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o
>> obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o
>> obj-$(CONFIG_SYS_I2C_STM32F7) += stm32f7_i2c.o
>> +obj-$(CONFIG_SUN6I_P2WI) += sun6i_p2wi.o
>
> please sort alphabetical.
I was sorting by file name -- this will be fixed by renaming the config
symbol.
Regards,
Samuel
>> obj-$(CONFIG_SYS_I2C_SYNQUACER) += synquacer_i2c.o
>> obj-$(CONFIG_SYS_I2C_TEGRA) += tegra_i2c.o
>> obj-$(CONFIG_SYS_I2C_UNIPHIER) += i2c-uniphier.o
>> diff --git a/drivers/i2c/sun6i_p2wi.c b/drivers/i2c/sun6i_p2wi.c
>> new file mode 100644
>> index 00000000000..f1e8e5ed107
>> --- /dev/null
>> +++ b/drivers/i2c/sun6i_p2wi.c
>> @@ -0,0 +1,66 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +
>> +#include <axp_pmic.h>
>> +#include <dm.h>
>> +#include <i2c.h>
>> +#include <asm/arch/p2wi.h>
>> +
>> +#if CONFIG_IS_ENABLED(DM_I2C)
>> +
>> +static int sun6i_p2wi_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
>> +{
>> + /* The hardware only supports SMBus-style transfers. */
>> + if (nmsgs == 2 && msg[1].flags == I2C_M_RD && msg[1].len == 1)
>> + return p2wi_read(msg[0].buf[0], msg[1].buf);
>> +
>> + if (nmsgs == 1 && msg[0].len == 2)
>> + return p2wi_write(msg[0].buf[0], msg[0].buf[1]);
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static int sun6i_p2wi_probe_chip(struct udevice *bus, uint chip_addr,
>> + uint chip_flags)
>> +{
>> + return p2wi_change_to_p2wi_mode(chip_addr,
>> + AXP_PMIC_MODE_REG,
>> + AXP_PMIC_MODE_P2WI);
>> +}
>> +
>> +static int sun6i_p2wi_probe(struct udevice *bus)
>> +{
>> + p2wi_init();
>> +
>> + return 0;
>> +}
>> +
>> +static int sun6i_p2wi_child_pre_probe(struct udevice *child)
>> +{
>> + struct dm_i2c_chip *chip = dev_get_parent_plat(child);
>> +
>> + /* Ensure each transfer is for a single register. */
>> + chip->flags |= DM_I2C_CHIP_RD_ADDRESS | DM_I2C_CHIP_WR_ADDRESS;
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dm_i2c_ops sun6i_p2wi_ops = {
>> + .xfer = sun6i_p2wi_xfer,
>> + .probe_chip = sun6i_p2wi_probe_chip,
>> +};
>> +
>> +static const struct udevice_id sun6i_p2wi_ids[] = {
>> + { .compatible = "allwinner,sun6i-a31-p2wi" },
>> + { /* sentinel */ }
>> +};
>> +
>> +U_BOOT_DRIVER(sun6i_p2wi) = {
>> + .name = "sun6i_p2wi",
>> + .id = UCLASS_I2C,
>> + .of_match = sun6i_p2wi_ids,
>> + .probe = sun6i_p2wi_probe,
>> + .child_pre_probe = sun6i_p2wi_child_pre_probe,
>> + .ops = &sun6i_p2wi_ops,
>> +};
>> +
>> +#endif /* CONFIG_IS_ENABLED(DM_I2C) */
>> diff --git a/include/axp_pmic.h b/include/axp_pmic.h
>> index 405044c3a32..0db3e143eda 100644
>> --- a/include/axp_pmic.h
>> +++ b/include/axp_pmic.h
>> @@ -6,6 +6,8 @@
>> */
>> #ifndef _AXP_PMIC_H_
>>
>> +#include <stdbool.h>
>> +
>> #ifdef CONFIG_AXP152_POWER
>> #include <axp152.h>
>> #endif
>> @@ -25,6 +27,10 @@
>> #include <axp818.h>
>> #endif
>>
>> +#define AXP_PMIC_MODE_REG 0x3e
>> +#define AXP_PMIC_MODE_I2C 0x00
>> +#define AXP_PMIC_MODE_P2WI 0x3e
>> +
>> int axp_set_dcdc1(unsigned int mvolt);
>> int axp_set_dcdc2(unsigned int mvolt);
>> int axp_set_dcdc3(unsigned int mvolt);
>>
>
> Beside of the nitpicks:
> Reviewed-by: Heiko Schocher <hs at denx.de>
>
> Thanks!
>
> bye,
> Heiko
>
More information about the U-Boot
mailing list