Re: [PATCH 04/13] soc: samsung: Add Exynos USI driver
On Wed, Dec 27, 2023 at 11:49 AM Simon Glass wrote:
>
> Hi Sam,
>
[snip]
>
> Just a few nits here
>
> Reviewed-by: Simon Glass
>
[snip]
> > +
> > +struct exynos_usi {
> > + struct udevice *dev;
>
> Can we drop this? It doesn't seem very useful and we try to avoid
> having bidirectional pointers. since it is possible to get the 'priv'
> pointer from the device.
>
Sure. I tried to keep the driver as close as possible to Linux
kernel's version, where I borrowed it from. But if it's the current
preference in U-Boot, I'll fix this in v2.
[snip]
> > +static int exynos_usi_parse_dt(struct exynos_usi *usi)
>
> Use of_to_plat() method?
>
Will do in v2. Thanks for the review!
[snip]
Re: [PATCH 04/13] soc: samsung: Add Exynos USI driver
On Wed, Dec 27, 2023 at 3:11 AM Minkyu Kang wrote:
>
> Hi
>
>
> 2023년 12월 13일 (수) 12:42, Sam Protsenko 님이 작성:
>>
[snip]
>> +
>> +/**
>> + * exynos_usi_set_sw_conf - Set USI block configuration mode
>> + * @usi: USI driver object
>> + * @mode: Mode index
>> + *
>> + * Select underlying serial protocol (UART/SPI/I2C) in USI IP-core.
>> + *
>> + * Return: 0 on success, or negative error code on failure.
>> + */
>> +static int exynos_usi_set_sw_conf(struct exynos_usi *usi, size_t mode)
>
>
> The value of mode is same as usi->mode, but is there a reason to pass it as a
> parameter?
>
>>
>> +{
>> + unsigned int val;
>> + int ret;
>> +
>> + if (mode < usi->data->min_mode || mode > usi->data->max_mode)
>> + return -EINVAL;
>> +
>> + val = exynos_usi_modes[mode].val;
>> + ret = regmap_update_bits(usi->sysreg, usi->sw_conf,
>> +usi->data->sw_conf_mask, val);
>> + if (ret)
>> + return ret;
>> +
>> + usi->mode = mode;
>
>
> This will obviously be the same value always.
>
Thanks for the review! Yes, you are right. This code was copy-pasted
from Linux kernel. So at the time I thought it was better to leave it
as is, for backporting reasons. But now that I look at it, this won't
be too helpful. So I'll get rid of it in v2.
[snip]
Re: [PATCH 04/13] soc: samsung: Add Exynos USI driver
Hi Sam, On Wed, Dec 13, 2023 at 3:16 AM Sam Protsenko wrote: > > USIv2 IP-core is found on modern ARM64 Exynos SoCs (like Exynos850) and > provides selectable serial protocol (one of: UART, SPI, I2C). USIv2 > registers usually reside in the same register map as a particular > underlying protocol it implements, but have some particular offset. E.g. > on Exynos850 the USI_UART has 0x1382 base address, where UART > registers have 0x00..0x40 offsets, and USI registers have 0xc0..0xdc > offsets. Desired protocol can be chosen via SW_CONF register from System > Register block of the same domain as USI. > > Before starting to use a particular protocol, USIv2 must be configured > properly: > 1. Select protocol to be used via System Register > 2. Clear "reset" flag in USI_CON > 3. Configure HWACG behavior (e.g. for UART Rx the HWACG must be > disabled, so that the IP clock is not gated automatically); this is > done using USI_OPTION register > 4. Keep both USI clocks (PCLK and IPCLK) running during USI registers > modification > > This driver implements the above behavior. Of course, USIv2 driver > should be probed before UART/I2C/SPI drivers. It can be achieved by > embedding UART/I2C/SPI nodes inside of the USI node (in Device Tree); > driver then walks underlying nodes and instantiates those. Driver also > handles USI configuration on PM resume, as register contents can be lost > during CPU suspend. > > This driver is designed with different USI versions in mind. So it > should be relatively easy to add new USI revisions to it later. > > Driver's code was copied over from Linux kernel [1] and adapted > correspondingly for U-Boot API. UCLASS_MISC is used, and although no > misc operations are implemented, it makes it easier to probe the driver > this way (as compared to UCLASS_NOP) and keep the code compact. > > [1] drivers/soc/samsung/exynos-usi.c > > Signed-off-by: Sam Protsenko > --- > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/samsung/Kconfig | 23 > drivers/soc/samsung/Makefile | 3 + > drivers/soc/samsung/exynos-usi.c | 218 +++ > 5 files changed, 246 insertions(+) > create mode 100644 drivers/soc/samsung/Kconfig > create mode 100644 drivers/soc/samsung/Makefile > create mode 100644 drivers/soc/samsung/exynos-usi.c > Just a few nits here Reviewed-by: Simon Glass > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig > index 85dac9de78a4..03433bc0e6d2 100644 > --- a/drivers/soc/Kconfig > +++ b/drivers/soc/Kconfig > @@ -40,6 +40,7 @@ config SOC_XILINX_VERSAL_NET > This allows other drivers to verify the SoC familiy & revision using > matching SoC attributes. > > +source "drivers/soc/samsung/Kconfig" > source "drivers/soc/ti/Kconfig" > > endmenu > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile > index 84385650d46d..610bf816d40a 100644 > --- a/drivers/soc/Makefile > +++ b/drivers/soc/Makefile > @@ -2,6 +2,7 @@ > # > # Makefile for the U-Boot SOC specific device drivers. > > +obj-$(CONFIG_SOC_SAMSUNG) += samsung/ > obj-$(CONFIG_SOC_TI) += ti/ > obj-$(CONFIG_SOC_DEVICE) += soc-uclass.o > obj-$(CONFIG_SOC_DEVICE_TI_K3) += soc_ti_k3.o > diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig > new file mode 100644 > index ..ffb87fe79316 > --- /dev/null > +++ b/drivers/soc/samsung/Kconfig > @@ -0,0 +1,23 @@ > +# SPDX-License-Identifier: GPL-2.0+ > + > +menuconfig SOC_SAMSUNG > + bool "Samsung SoC drivers support" > + > +if SOC_SAMSUNG > + > +config EXYNOS_USI > + bool "Exynos USI (Universal Serial Interface) driver" > + depends on ARCH_EXYNOS > + select MISC > + select REGMAP > + select SYSCON > + help > + Enable support for USI block. USI (Universal Serial Interface) is an > + IP-core found in modern Samsung Exynos SoCs, like Exynos850 and > + ExynosAutoV9. USI block can be configured to provide one of the > + following serial protocols: UART, SPI or High Speed I2C. > + > + This driver allows one to configure USI for desired protocol, which > + is usually done in USI node in Device Tree. > + > +endif > diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile > new file mode 100644 > index ..833ac073fbfa > --- /dev/null > +++ b/drivers/soc/samsung/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0+ > + > +obj-$(CONFIG_EXYNOS_USI) += exynos-usi.o > diff --git a/drivers/soc/samsung/exynos-usi.c > b/drivers/soc/samsung/exynos-usi.c > new file mode 100644 > index ..23255177e6e3 > --- /dev/null > +++ b/drivers/soc/samsung/exynos-usi.c > @@ -0,0 +1,218 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2023 Linaro Ltd. > + * Author: Sam Protsenko > + * > + * Samsung Exynos USI driver (Universal Serial Interface). > + */ > + > +#include > +#include > +#
Re: [PATCH 04/13] soc: samsung: Add Exynos USI driver
Hi 2023년 12월 13일 (수) 12:42, Sam Protsenko 님이 작성: > USIv2 IP-core is found on modern ARM64 Exynos SoCs (like Exynos850) and > provides selectable serial protocol (one of: UART, SPI, I2C). USIv2 > registers usually reside in the same register map as a particular > underlying protocol it implements, but have some particular offset. E.g. > on Exynos850 the USI_UART has 0x1382 base address, where UART > registers have 0x00..0x40 offsets, and USI registers have 0xc0..0xdc > offsets. Desired protocol can be chosen via SW_CONF register from System > Register block of the same domain as USI. > > Before starting to use a particular protocol, USIv2 must be configured > properly: > 1. Select protocol to be used via System Register > 2. Clear "reset" flag in USI_CON > 3. Configure HWACG behavior (e.g. for UART Rx the HWACG must be > disabled, so that the IP clock is not gated automatically); this is > done using USI_OPTION register > 4. Keep both USI clocks (PCLK and IPCLK) running during USI registers > modification > > This driver implements the above behavior. Of course, USIv2 driver > should be probed before UART/I2C/SPI drivers. It can be achieved by > embedding UART/I2C/SPI nodes inside of the USI node (in Device Tree); > driver then walks underlying nodes and instantiates those. Driver also > handles USI configuration on PM resume, as register contents can be lost > during CPU suspend. > > This driver is designed with different USI versions in mind. So it > should be relatively easy to add new USI revisions to it later. > > Driver's code was copied over from Linux kernel [1] and adapted > correspondingly for U-Boot API. UCLASS_MISC is used, and although no > misc operations are implemented, it makes it easier to probe the driver > this way (as compared to UCLASS_NOP) and keep the code compact. > > [1] drivers/soc/samsung/exynos-usi.c > > Signed-off-by: Sam Protsenko > --- > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/samsung/Kconfig | 23 > drivers/soc/samsung/Makefile | 3 + > drivers/soc/samsung/exynos-usi.c | 218 +++ > 5 files changed, 246 insertions(+) > create mode 100644 drivers/soc/samsung/Kconfig > create mode 100644 drivers/soc/samsung/Makefile > create mode 100644 drivers/soc/samsung/exynos-usi.c > > diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig > index 85dac9de78a4..03433bc0e6d2 100644 > --- a/drivers/soc/Kconfig > +++ b/drivers/soc/Kconfig > @@ -40,6 +40,7 @@ config SOC_XILINX_VERSAL_NET > This allows other drivers to verify the SoC familiy & revision > using > matching SoC attributes. > > +source "drivers/soc/samsung/Kconfig" > source "drivers/soc/ti/Kconfig" > > endmenu > diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile > index 84385650d46d..610bf816d40a 100644 > --- a/drivers/soc/Makefile > +++ b/drivers/soc/Makefile > @@ -2,6 +2,7 @@ > # > # Makefile for the U-Boot SOC specific device drivers. > > +obj-$(CONFIG_SOC_SAMSUNG) += samsung/ > obj-$(CONFIG_SOC_TI) += ti/ > obj-$(CONFIG_SOC_DEVICE) += soc-uclass.o > obj-$(CONFIG_SOC_DEVICE_TI_K3) += soc_ti_k3.o > diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig > new file mode 100644 > index ..ffb87fe79316 > --- /dev/null > +++ b/drivers/soc/samsung/Kconfig > @@ -0,0 +1,23 @@ > +# SPDX-License-Identifier: GPL-2.0+ > + > +menuconfig SOC_SAMSUNG > + bool "Samsung SoC drivers support" > + > +if SOC_SAMSUNG > + > +config EXYNOS_USI > + bool "Exynos USI (Universal Serial Interface) driver" > + depends on ARCH_EXYNOS > + select MISC > + select REGMAP > + select SYSCON > + help > + Enable support for USI block. USI (Universal Serial Interface) > is an > + IP-core found in modern Samsung Exynos SoCs, like Exynos850 and > + ExynosAutoV9. USI block can be configured to provide one of the > + following serial protocols: UART, SPI or High Speed I2C. > + > + This driver allows one to configure USI for desired protocol, > which > + is usually done in USI node in Device Tree. > + > +endif > diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile > new file mode 100644 > index ..833ac073fbfa > --- /dev/null > +++ b/drivers/soc/samsung/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0+ > + > +obj-$(CONFIG_EXYNOS_USI) += exynos-usi.o > diff --git a/drivers/soc/samsung/exynos-usi.c > b/drivers/soc/samsung/exynos-usi.c > new file mode 100644 > index ..23255177e6e3 > --- /dev/null > +++ b/drivers/soc/samsung/exynos-usi.c > @@ -0,0 +1,218 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2023 Linaro Ltd. > + * Author: Sam Protsenko > + * > + * Samsung Exynos USI driver (Universal Serial Interface). > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#includ
RE: [PATCH 04/13] soc: samsung: Add Exynos USI driver
> -Original Message- > From: U-Boot On Behalf Of Sam Protsenko > Sent: Wednesday, December 13, 2023 12:17 PM > To: Minkyu Kang ; Tom Rini ; > Lukasz Majewski ; Sean Anderson > Cc: Simon Glass ; Heinrich Schuchardt > ; u-boot@lists.denx.de > Subject: [PATCH 04/13] soc: samsung: Add Exynos USI driver > > USIv2 IP-core is found on modern ARM64 Exynos SoCs (like Exynos850) and > provides selectable serial protocol (one of: UART, SPI, I2C). USIv2 > registers usually reside in the same register map as a particular > underlying protocol it implements, but have some particular offset. E.g. > on Exynos850 the USI_UART has 0x1382 base address, where UART > registers have 0x00..0x40 offsets, and USI registers have 0xc0..0xdc > offsets. Desired protocol can be chosen via SW_CONF register from System > Register block of the same domain as USI. > > Before starting to use a particular protocol, USIv2 must be configured > properly: > 1. Select protocol to be used via System Register > 2. Clear "reset" flag in USI_CON > 3. Configure HWACG behavior (e.g. for UART Rx the HWACG must be > disabled, so that the IP clock is not gated automatically); this is > done using USI_OPTION register > 4. Keep both USI clocks (PCLK and IPCLK) running during USI registers > modification > > This driver implements the above behavior. Of course, USIv2 driver > should be probed before UART/I2C/SPI drivers. It can be achieved by > embedding UART/I2C/SPI nodes inside of the USI node (in Device Tree); > driver then walks underlying nodes and instantiates those. Driver also > handles USI configuration on PM resume, as register contents can be lost > during CPU suspend. > > This driver is designed with different USI versions in mind. So it > should be relatively easy to add new USI revisions to it later. > > Driver's code was copied over from Linux kernel [1] and adapted > correspondingly for U-Boot API. UCLASS_MISC is used, and although no > misc operations are implemented, it makes it easier to probe the driver > this way (as compared to UCLASS_NOP) and keep the code compact. > > [1] drivers/soc/samsung/exynos-usi.c > > Signed-off-by: Sam Protsenko Reviewed-by: Chanho Park
