Re: [PATCH 1/6] misc: add Qualcomm GENI SE QUP device driver
Hi Simon, On 4/19/23 04:45, Simon Glass wrote: Hi Vladimir, On Wed, 12 Apr 2023 at 06:45, Vladimir Zapolskiy wrote: On 3/31/23 04:23, Konrad Dybcio wrote: On 30.03.2023 21:47, Vladimir Zapolskiy wrote: This change adds a Qualcomm GENI SE QUP device driver as a wrapper for actually enabled and used serial devices found on a board. At the moment the driver is pretty simple, its intention is to populate childred devices and provide I/O mem read interface to them as clients, this is needed for GENI UART driver to set up a proper clock divider and provide the actually asked baud rate. Signed-off-by: Vladimir Zapolskiy --- drivers/misc/Kconfig| 6 ++ drivers/misc/Makefile | 1 + drivers/misc/qcom-geni-se.c | 42 + 3 files changed, 49 insertions(+) create mode 100644 drivers/misc/qcom-geni-se.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index b5707a15c504..348e1ab407ad 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -511,6 +511,12 @@ config WINBOND_W83627 legacy UART or other devices in the Winbond Super IO chips on X86 platforms. +config QCOM_GENI_SE +bool "Qualcomm GENI Serial Engine Driver" +help + The driver manages Generic Interface (GENI) firmware based + Qualcomm Technologies, Inc. Universal Peripheral (QUP) Wrapper. + config QFW bool help diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 3b792f2a14ce..52aed096021f 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o obj-$(CONFIG_P2SB) += p2sb-uclass.o obj-$(CONFIG_PCA9551_LED) += pca9551_led.o obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o +obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o ifdef CONFIG_QFW obj-y += qfw.o obj-$(CONFIG_QFW_PIO) += qfw_pio.o diff --git a/drivers/misc/qcom-geni-se.c b/drivers/misc/qcom-geni-se.c new file mode 100644 index ..4f1775b11f62 --- /dev/null +++ b/drivers/misc/qcom-geni-se.c @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Qualcomm Generic Interface (GENI) Serial Engine (SE) Wrapper + * + * (C) Copyright 2023 Vladimir Zapolskiy + */ + +#include +#include +#include +#include + +static int geni_se_qup_read(struct udevice *dev, int offset, +void *buf, int size) +{ +fdt_addr_t base = dev_read_addr(dev); + +if (size != sizeof(u32)) +return -EINVAL; + +*(u32 *)buf = readl(base + offset); Maybe u32 *buffer = buf; [...] *buf = readl.. Well, I'd rather keep it as is, since it's one code of line less. would be more stylish, but that's a nit and I don't have any other complaints! :D In fact there is a minor issue with the driver, namely after some testing I have to remove '.bind = dm_scan_fdt_dev' from the ops, since it's already done on an upper level. Reviewed-by: Konrad Dybcio Thanks for review! Best wishes, Vladimir + +return 0; +} + +static struct misc_ops geni_se_qup_ops = { +.read = geni_se_qup_read, +}; + +static const struct udevice_id geni_se_qup_ids[] = { +{ .compatible = "qcom,geni-se-qup" }, +{} +}; + +U_BOOT_DRIVER(geni_se_qup) = { +.name = "geni_se_qup", +.id = UCLASS_MISC, +.of_match = geni_se_qup_ids, +.bind = dm_scan_fdt_dev, +.ops = _se_qup_ops, +.flags = DM_FLAG_PRE_RELOC, +}; Please note that misc_read() should return the number of bytes read, not 0. oh, definitely this should be fixed, and consequently it requires v3 to be sent. Thank you for review! -- Best wishes, Vladimir
Re: [PATCH 1/6] misc: add Qualcomm GENI SE QUP device driver
Hi Vladimir, On Wed, 12 Apr 2023 at 06:45, Vladimir Zapolskiy wrote: > > On 3/31/23 04:23, Konrad Dybcio wrote: > > > > > > On 30.03.2023 21:47, Vladimir Zapolskiy wrote: > >> This change adds a Qualcomm GENI SE QUP device driver as a wrapper for > >> actually enabled and used serial devices found on a board. > >> > >> At the moment the driver is pretty simple, its intention is to populate > >> childred devices and provide I/O mem read interface to them as clients, > >> this is needed for GENI UART driver to set up a proper clock divider > >> and provide the actually asked baud rate. > >> > >> Signed-off-by: Vladimir Zapolskiy > >> --- > >> drivers/misc/Kconfig| 6 ++ > >> drivers/misc/Makefile | 1 + > >> drivers/misc/qcom-geni-se.c | 42 + > >> 3 files changed, 49 insertions(+) > >> create mode 100644 drivers/misc/qcom-geni-se.c > >> > >> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > >> index b5707a15c504..348e1ab407ad 100644 > >> --- a/drivers/misc/Kconfig > >> +++ b/drivers/misc/Kconfig > >> @@ -511,6 +511,12 @@ config WINBOND_W83627 > >>legacy UART or other devices in the Winbond Super IO chips > >>on X86 platforms. > >> > >> +config QCOM_GENI_SE > >> +bool "Qualcomm GENI Serial Engine Driver" > >> +help > >> + The driver manages Generic Interface (GENI) firmware based > >> + Qualcomm Technologies, Inc. Universal Peripheral (QUP) Wrapper. > >> + > >> config QFW > >> bool > >> help > >> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > >> index 3b792f2a14ce..52aed096021f 100644 > >> --- a/drivers/misc/Makefile > >> +++ b/drivers/misc/Makefile > >> @@ -60,6 +60,7 @@ obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o > >> obj-$(CONFIG_P2SB) += p2sb-uclass.o > >> obj-$(CONFIG_PCA9551_LED) += pca9551_led.o > >> obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o > >> +obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o > >> ifdef CONFIG_QFW > >> obj-y += qfw.o > >> obj-$(CONFIG_QFW_PIO) += qfw_pio.o > >> diff --git a/drivers/misc/qcom-geni-se.c b/drivers/misc/qcom-geni-se.c > >> new file mode 100644 > >> index ..4f1775b11f62 > >> --- /dev/null > >> +++ b/drivers/misc/qcom-geni-se.c > >> @@ -0,0 +1,42 @@ > >> +// SPDX-License-Identifier: GPL-2.0+ > >> +/* > >> + * Qualcomm Generic Interface (GENI) Serial Engine (SE) Wrapper > >> + * > >> + * (C) Copyright 2023 Vladimir Zapolskiy > >> + */ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> + > >> +static int geni_se_qup_read(struct udevice *dev, int offset, > >> +void *buf, int size) > >> +{ > >> +fdt_addr_t base = dev_read_addr(dev); > >> + > >> +if (size != sizeof(u32)) > >> +return -EINVAL; > >> + > >> +*(u32 *)buf = readl(base + offset); > > Maybe > > > > u32 *buffer = buf; > > > > [...] > > > > *buf = readl.. > > Well, I'd rather keep it as is, since it's one code of line less. > > > would be more stylish, but that's a nit and I don't have > > any other complaints! :D > > In fact there is a minor issue with the driver, namely after some > testing I have to remove '.bind = dm_scan_fdt_dev' from the ops, > since it's already done on an upper level. > > > Reviewed-by: Konrad Dybcio > > > > Thanks for review! > > Best wishes, > Vladimir > > >> + > >> +return 0; > >> +} > >> + > >> +static struct misc_ops geni_se_qup_ops = { > >> +.read = geni_se_qup_read, > >> +}; > >> + > >> +static const struct udevice_id geni_se_qup_ids[] = { > >> +{ .compatible = "qcom,geni-se-qup" }, > >> +{} > >> +}; > >> + > >> +U_BOOT_DRIVER(geni_se_qup) = { > >> +.name = "geni_se_qup", > >> +.id = UCLASS_MISC, > >> +.of_match = geni_se_qup_ids, > >> +.bind = dm_scan_fdt_dev, > >> +.ops = _se_qup_ops, > >> +.flags = DM_FLAG_PRE_RELOC, > >> +}; Please note that misc_read() should return the number of bytes read, not 0. Regards, Simon
Re: [PATCH 1/6] misc: add Qualcomm GENI SE QUP device driver
On 3/31/23 04:23, Konrad Dybcio wrote: On 30.03.2023 21:47, Vladimir Zapolskiy wrote: This change adds a Qualcomm GENI SE QUP device driver as a wrapper for actually enabled and used serial devices found on a board. At the moment the driver is pretty simple, its intention is to populate childred devices and provide I/O mem read interface to them as clients, this is needed for GENI UART driver to set up a proper clock divider and provide the actually asked baud rate. Signed-off-by: Vladimir Zapolskiy --- drivers/misc/Kconfig| 6 ++ drivers/misc/Makefile | 1 + drivers/misc/qcom-geni-se.c | 42 + 3 files changed, 49 insertions(+) create mode 100644 drivers/misc/qcom-geni-se.c diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index b5707a15c504..348e1ab407ad 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -511,6 +511,12 @@ config WINBOND_W83627 legacy UART or other devices in the Winbond Super IO chips on X86 platforms. +config QCOM_GENI_SE + bool "Qualcomm GENI Serial Engine Driver" + help + The driver manages Generic Interface (GENI) firmware based + Qualcomm Technologies, Inc. Universal Peripheral (QUP) Wrapper. + config QFW bool help diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 3b792f2a14ce..52aed096021f 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o obj-$(CONFIG_P2SB) += p2sb-uclass.o obj-$(CONFIG_PCA9551_LED) += pca9551_led.o obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o +obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o ifdef CONFIG_QFW obj-y += qfw.o obj-$(CONFIG_QFW_PIO) += qfw_pio.o diff --git a/drivers/misc/qcom-geni-se.c b/drivers/misc/qcom-geni-se.c new file mode 100644 index ..4f1775b11f62 --- /dev/null +++ b/drivers/misc/qcom-geni-se.c @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Qualcomm Generic Interface (GENI) Serial Engine (SE) Wrapper + * + * (C) Copyright 2023 Vladimir Zapolskiy + */ + +#include +#include +#include +#include + +static int geni_se_qup_read(struct udevice *dev, int offset, + void *buf, int size) +{ + fdt_addr_t base = dev_read_addr(dev); + + if (size != sizeof(u32)) + return -EINVAL; + + *(u32 *)buf = readl(base + offset); Maybe u32 *buffer = buf; [...] *buf = readl.. Well, I'd rather keep it as is, since it's one code of line less. would be more stylish, but that's a nit and I don't have any other complaints! :D In fact there is a minor issue with the driver, namely after some testing I have to remove '.bind = dm_scan_fdt_dev' from the ops, since it's already done on an upper level. Reviewed-by: Konrad Dybcio Thanks for review! Best wishes, Vladimir + + return 0; +} + +static struct misc_ops geni_se_qup_ops = { + .read = geni_se_qup_read, +}; + +static const struct udevice_id geni_se_qup_ids[] = { + { .compatible = "qcom,geni-se-qup" }, + {} +}; + +U_BOOT_DRIVER(geni_se_qup) = { + .name = "geni_se_qup", + .id = UCLASS_MISC, + .of_match = geni_se_qup_ids, + .bind = dm_scan_fdt_dev, + .ops = _se_qup_ops, + .flags = DM_FLAG_PRE_RELOC, +};
Re: [PATCH 1/6] misc: add Qualcomm GENI SE QUP device driver
On 30.03.2023 21:47, Vladimir Zapolskiy wrote: > This change adds a Qualcomm GENI SE QUP device driver as a wrapper for > actually enabled and used serial devices found on a board. > > At the moment the driver is pretty simple, its intention is to populate > childred devices and provide I/O mem read interface to them as clients, > this is needed for GENI UART driver to set up a proper clock divider > and provide the actually asked baud rate. > > Signed-off-by: Vladimir Zapolskiy > --- > drivers/misc/Kconfig| 6 ++ > drivers/misc/Makefile | 1 + > drivers/misc/qcom-geni-se.c | 42 + > 3 files changed, 49 insertions(+) > create mode 100644 drivers/misc/qcom-geni-se.c > > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index b5707a15c504..348e1ab407ad 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -511,6 +511,12 @@ config WINBOND_W83627 > legacy UART or other devices in the Winbond Super IO chips > on X86 platforms. > > +config QCOM_GENI_SE > + bool "Qualcomm GENI Serial Engine Driver" > + help > + The driver manages Generic Interface (GENI) firmware based > + Qualcomm Technologies, Inc. Universal Peripheral (QUP) Wrapper. > + > config QFW > bool > help > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index 3b792f2a14ce..52aed096021f 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -60,6 +60,7 @@ obj-$(CONFIG_NUVOTON_NCT6102D) += nuvoton_nct6102d.o > obj-$(CONFIG_P2SB) += p2sb-uclass.o > obj-$(CONFIG_PCA9551_LED) += pca9551_led.o > obj-$(CONFIG_$(SPL_)PWRSEQ) += pwrseq-uclass.o > +obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o > ifdef CONFIG_QFW > obj-y += qfw.o > obj-$(CONFIG_QFW_PIO) += qfw_pio.o > diff --git a/drivers/misc/qcom-geni-se.c b/drivers/misc/qcom-geni-se.c > new file mode 100644 > index ..4f1775b11f62 > --- /dev/null > +++ b/drivers/misc/qcom-geni-se.c > @@ -0,0 +1,42 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Qualcomm Generic Interface (GENI) Serial Engine (SE) Wrapper > + * > + * (C) Copyright 2023 Vladimir Zapolskiy > + */ > + > +#include > +#include > +#include > +#include > + > +static int geni_se_qup_read(struct udevice *dev, int offset, > + void *buf, int size) > +{ > + fdt_addr_t base = dev_read_addr(dev); > + > + if (size != sizeof(u32)) > + return -EINVAL; > + > + *(u32 *)buf = readl(base + offset); Maybe u32 *buffer = buf; [...] *buf = readl.. would be more stylish, but that's a nit and I don't have any other complaints! :D Reviewed-by: Konrad Dybcio Konrad > + > + return 0; > +} > + > +static struct misc_ops geni_se_qup_ops = { > + .read = geni_se_qup_read, > +}; > + > +static const struct udevice_id geni_se_qup_ids[] = { > + { .compatible = "qcom,geni-se-qup" }, > + {} > +}; > + > +U_BOOT_DRIVER(geni_se_qup) = { > + .name = "geni_se_qup", > + .id = UCLASS_MISC, > + .of_match = geni_se_qup_ids, > + .bind = dm_scan_fdt_dev, > + .ops = _se_qup_ops, > + .flags = DM_FLAG_PRE_RELOC, > +};