| Message ID | 20230330194736.2400593-2-vladimir.zapolskiy@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | serial: msm-geni: fix UART baudrate on modern platforms | expand |
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 <vladimir.zapolskiy@linaro.org> > --- > 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 000000000000..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 <vladimir.zapolskiy@linaro.org> > + */ > + > +#include <common.h> > +#include <dm.h> > +#include <misc.h> > +#include <asm/io.h> > + > +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.dybcio@linaro.org> 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 = &geni_se_qup_ops, > + .flags = DM_FLAG_PRE_RELOC, > +};
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 <vladimir.zapolskiy@linaro.org> >> --- >> 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 000000000000..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 <vladimir.zapolskiy@linaro.org> >> + */ >> + >> +#include <common.h> >> +#include <dm.h> >> +#include <misc.h> >> +#include <asm/io.h> >> + >> +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 <konrad.dybcio@linaro.org> > 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 = &geni_se_qup_ops, >> + .flags = DM_FLAG_PRE_RELOC, >> +};
Hi Vladimir, On Wed, 12 Apr 2023 at 06:45, Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> 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 <vladimir.zapolskiy@linaro.org> > >> --- > >> 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 000000000000..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 <vladimir.zapolskiy@linaro.org> > >> + */ > >> + > >> +#include <common.h> > >> +#include <dm.h> > >> +#include <misc.h> > >> +#include <asm/io.h> > >> + > >> +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 <konrad.dybcio@linaro.org> > > > > 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 = &geni_se_qup_ops, > >> + .flags = DM_FLAG_PRE_RELOC, > >> +}; Please note that misc_read() should return the number of bytes read, not 0. Regards, Simon
Hi Simon, On 4/19/23 04:45, Simon Glass wrote: > Hi Vladimir, > > On Wed, 12 Apr 2023 at 06:45, Vladimir Zapolskiy > <vladimir.zapolskiy@linaro.org> 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 <vladimir.zapolskiy@linaro.org> >>>> --- >>>> 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 000000000000..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 <vladimir.zapolskiy@linaro.org> >>>> + */ >>>> + >>>> +#include <common.h> >>>> +#include <dm.h> >>>> +#include <misc.h> >>>> +#include <asm/io.h> >>>> + >>>> +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 <konrad.dybcio@linaro.org> >>> >> >> 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 = &geni_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
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 000000000000..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 <vladimir.zapolskiy@linaro.org> + */ + +#include <common.h> +#include <dm.h> +#include <misc.h> +#include <asm/io.h> + +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); + + 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 = &geni_se_qup_ops, + .flags = DM_FLAG_PRE_RELOC, +};
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 <vladimir.zapolskiy@linaro.org> --- 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