| Message ID | 20240715-b4-qcom-rpmh-v6-6-0c948a25d018@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qcom: rpmh core and regulator support | expand |
On Mon, 15 Jul 2024 at 11:08, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > Integrate cmd-db into the U-Boot driver model. > > This is just a wrapper around an in-memory database, so we just need to > get the address and validate that cmd-db is there. > > Since cmd_db_header will be stored in the .data section we can skip > bind if it's already set. > > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > To: Simon Glass <sjg@chromium.org> > --- > drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- > include/soc/qcom/cmd-db.h | 3 -- > 2 files changed, 23 insertions(+), 52 deletions(-) > Reviewed-by: Simon Glass <sjg@chromium.org> BTW, this is a MISC driver. Does it implement the read() operation?
On 15/07/2024 13:39, Simon Glass wrote: > On Mon, 15 Jul 2024 at 11:08, Caleb Connolly <caleb.connolly@linaro.org> wrote: >> >> Integrate cmd-db into the U-Boot driver model. >> >> This is just a wrapper around an in-memory database, so we just need to >> get the address and validate that cmd-db is there. >> >> Since cmd_db_header will be stored in the .data section we can skip >> bind if it's already set. >> >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >> --- >> To: Simon Glass <sjg@chromium.org> >> --- >> drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- >> include/soc/qcom/cmd-db.h | 3 -- >> 2 files changed, 23 insertions(+), 52 deletions(-) >> > > Reviewed-by: Simon Glass <sjg@chromium.org> Thanks > > BTW, this is a MISC driver. Does it implement the read() operation? No, the API (as defined in the header file) takes a string and returns a database entry. cmd-db users don't have an easily available handle to the node / udevice regardless.
Hi Caleb, On Mon, 15 Jul 2024 at 22:42, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > > > On 15/07/2024 13:39, Simon Glass wrote: > > On Mon, 15 Jul 2024 at 11:08, Caleb Connolly <caleb.connolly@linaro.org> wrote: > >> > >> Integrate cmd-db into the U-Boot driver model. > >> > >> This is just a wrapper around an in-memory database, so we just need to > >> get the address and validate that cmd-db is there. > >> > >> Since cmd_db_header will be stored in the .data section we can skip > >> bind if it's already set. > >> > >> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > >> --- > >> To: Simon Glass <sjg@chromium.org> > >> --- > >> drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- > >> include/soc/qcom/cmd-db.h | 3 -- > >> 2 files changed, 23 insertions(+), 52 deletions(-) > >> > > > > Reviewed-by: Simon Glass <sjg@chromium.org> > > Thanks > > > > BTW, this is a MISC driver. Does it implement the read() operation? > > No, the API (as defined in the header file) takes a string and returns a > database entry. cmd-db users don't have an easily available handle to > the node / udevice regardless. OK. The closest thing might be UCLASS_SYSINFO, but it is designed for inside U-Boot and uses an int instead of a string for lookup. Could you use the devicetree for this information? What sort of info is it? Regards, Simon
On 16/07/2024 09:04, Simon Glass wrote: > Hi Caleb, > > On Mon, 15 Jul 2024 at 22:42, Caleb Connolly <caleb.connolly@linaro.org> wrote: >> >> >> >> On 15/07/2024 13:39, Simon Glass wrote: >>> On Mon, 15 Jul 2024 at 11:08, Caleb Connolly <caleb.connolly@linaro.org> wrote: >>>> >>>> Integrate cmd-db into the U-Boot driver model. >>>> >>>> This is just a wrapper around an in-memory database, so we just need to >>>> get the address and validate that cmd-db is there. >>>> >>>> Since cmd_db_header will be stored in the .data section we can skip >>>> bind if it's already set. >>>> >>>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >>>> --- >>>> To: Simon Glass <sjg@chromium.org> >>>> --- >>>> drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- >>>> include/soc/qcom/cmd-db.h | 3 -- >>>> 2 files changed, 23 insertions(+), 52 deletions(-) >>>> >>> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> Thanks >>> >>> BTW, this is a MISC driver. Does it implement the read() operation? >> >> No, the API (as defined in the header file) takes a string and returns a >> database entry. cmd-db users don't have an easily available handle to >> the node / udevice regardless. > > OK. The closest thing might be UCLASS_SYSINFO, but it is designed for > inside U-Boot and uses an int instead of a string for lookup. Right, I don't think we'll be able to find a more optimum solution here. > > Could you use the devicetree for this information? What sort of info is it? It maps resource names (e.g. "ldoa1" - the LDO1 regulator on PMIC A) to the address of the resource on the RPMh co-processor. We need to support existing (upstream) devicetree, since this has already been around for many years. > > Regards, > Simon
Hi Caleb, On Tue, 16 Jul 2024 at 08:16, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > > > On 16/07/2024 09:04, Simon Glass wrote: > > Hi Caleb, > > > > On Mon, 15 Jul 2024 at 22:42, Caleb Connolly <caleb.connolly@linaro.org> wrote: > >> > >> > >> > >> On 15/07/2024 13:39, Simon Glass wrote: > >>> On Mon, 15 Jul 2024 at 11:08, Caleb Connolly <caleb.connolly@linaro.org> wrote: > >>>> > >>>> Integrate cmd-db into the U-Boot driver model. > >>>> > >>>> This is just a wrapper around an in-memory database, so we just need to > >>>> get the address and validate that cmd-db is there. > >>>> > >>>> Since cmd_db_header will be stored in the .data section we can skip > >>>> bind if it's already set. > >>>> > >>>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > >>>> --- > >>>> To: Simon Glass <sjg@chromium.org> > >>>> --- > >>>> drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- > >>>> include/soc/qcom/cmd-db.h | 3 -- > >>>> 2 files changed, 23 insertions(+), 52 deletions(-) > >>>> > >>> > >>> Reviewed-by: Simon Glass <sjg@chromium.org> > >> > >> Thanks > >>> > >>> BTW, this is a MISC driver. Does it implement the read() operation? > >> > >> No, the API (as defined in the header file) takes a string and returns a > >> database entry. cmd-db users don't have an easily available handle to > >> the node / udevice regardless. > > > > OK. The closest thing might be UCLASS_SYSINFO, but it is designed for > > inside U-Boot and uses an int instead of a string for lookup. > > Right, I don't think we'll be able to find a more optimum solution here. OK > > > > Could you use the devicetree for this information? What sort of info is it? > > It maps resource names (e.g. "ldoa1" - the LDO1 regulator on PMIC A) to > the address of the resource on the RPMh co-processor. > > We need to support existing (upstream) devicetree, since this has > already been around for many years. OK., Has this problem been resolved in more recent platforms? Regards, Simon
On 19/07/2024 17:04, Simon Glass wrote: > Hi Caleb, > > On Tue, 16 Jul 2024 at 08:16, Caleb Connolly <caleb.connolly@linaro.org> wrote: >> >> >> >> On 16/07/2024 09:04, Simon Glass wrote: >>> Hi Caleb, >>> >>> On Mon, 15 Jul 2024 at 22:42, Caleb Connolly <caleb.connolly@linaro.org> wrote: >>>> >>>> >>>> >>>> On 15/07/2024 13:39, Simon Glass wrote: >>>>> On Mon, 15 Jul 2024 at 11:08, Caleb Connolly <caleb.connolly@linaro.org> wrote: >>>>>> >>>>>> Integrate cmd-db into the U-Boot driver model. >>>>>> >>>>>> This is just a wrapper around an in-memory database, so we just need to >>>>>> get the address and validate that cmd-db is there. >>>>>> >>>>>> Since cmd_db_header will be stored in the .data section we can skip >>>>>> bind if it's already set. >>>>>> >>>>>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> >>>>>> --- >>>>>> To: Simon Glass <sjg@chromium.org> >>>>>> --- >>>>>> drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- >>>>>> include/soc/qcom/cmd-db.h | 3 -- >>>>>> 2 files changed, 23 insertions(+), 52 deletions(-) >>>>>> >>>>> >>>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>> >>>> Thanks >>>>> >>>>> BTW, this is a MISC driver. Does it implement the read() operation? >>>> >>>> No, the API (as defined in the header file) takes a string and returns a >>>> database entry. cmd-db users don't have an easily available handle to >>>> the node / udevice regardless. >>> >>> OK. The closest thing might be UCLASS_SYSINFO, but it is designed for >>> inside U-Boot and uses an int instead of a string for lookup. >> >> Right, I don't think we'll be able to find a more optimum solution here. > > OK > >>> >>> Could you use the devicetree for this information? What sort of info is it? >> >> It maps resource names (e.g. "ldoa1" - the LDO1 regulator on PMIC A) to >> the address of the resource on the RPMh co-processor. >> >> We need to support existing (upstream) devicetree, since this has >> already been around for many years. > > OK., Has this problem been resolved in more recent platforms? What problem? > > Regards, > Simon
Hi Caleb, On Fri, 19 Jul 2024 at 20:45, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > > > On 19/07/2024 17:04, Simon Glass wrote: > > Hi Caleb, > > > > On Tue, 16 Jul 2024 at 08:16, Caleb Connolly <caleb.connolly@linaro.org> wrote: > >> > >> > >> > >> On 16/07/2024 09:04, Simon Glass wrote: > >>> Hi Caleb, > >>> > >>> On Mon, 15 Jul 2024 at 22:42, Caleb Connolly <caleb.connolly@linaro.org> wrote: > >>>> > >>>> > >>>> > >>>> On 15/07/2024 13:39, Simon Glass wrote: > >>>>> On Mon, 15 Jul 2024 at 11:08, Caleb Connolly <caleb.connolly@linaro.org> wrote: > >>>>>> > >>>>>> Integrate cmd-db into the U-Boot driver model. > >>>>>> > >>>>>> This is just a wrapper around an in-memory database, so we just need to > >>>>>> get the address and validate that cmd-db is there. > >>>>>> > >>>>>> Since cmd_db_header will be stored in the .data section we can skip > >>>>>> bind if it's already set. > >>>>>> > >>>>>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> > >>>>>> --- > >>>>>> To: Simon Glass <sjg@chromium.org> > >>>>>> --- > >>>>>> drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- > >>>>>> include/soc/qcom/cmd-db.h | 3 -- > >>>>>> 2 files changed, 23 insertions(+), 52 deletions(-) > >>>>>> > >>>>> > >>>>> Reviewed-by: Simon Glass <sjg@chromium.org> > >>>> > >>>> Thanks > >>>>> > >>>>> BTW, this is a MISC driver. Does it implement the read() operation? > >>>> > >>>> No, the API (as defined in the header file) takes a string and returns a > >>>> database entry. cmd-db users don't have an easily available handle to > >>>> the node / udevice regardless. > >>> > >>> OK. The closest thing might be UCLASS_SYSINFO, but it is designed for > >>> inside U-Boot and uses an int instead of a string for lookup. > >> > >> Right, I don't think we'll be able to find a more optimum solution here. > > > > OK > > > >>> > >>> Could you use the devicetree for this information? What sort of info is it? > >> > >> It maps resource names (e.g. "ldoa1" - the LDO1 regulator on PMIC A) to > >> the address of the resource on the RPMh co-processor. > >> > >> We need to support existing (upstream) devicetree, since this has > >> already been around for many years. > > > > OK., Has this problem been resolved in more recent platforms? > > What problem? Well it sounds like the board is using some sort of firmware service to obtain configuration when it should be using the devicetree. Is that right? Regards, Simon
diff --git a/drivers/soc/qcom/cmd-db.c b/drivers/soc/qcom/cmd-db.c index 4317310d0bcd..f707aed59adf 100644 --- a/drivers/soc/qcom/cmd-db.c +++ b/drivers/soc/qcom/cmd-db.c @@ -105,9 +105,9 @@ static bool cmd_db_magic_matches(const struct cmd_db_header *header) return memcmp(magic, CMD_DB_MAGIC, ARRAY_SIZE(CMD_DB_MAGIC)) == 0; } -static struct cmd_db_header *cmd_db_header; +static struct cmd_db_header *cmd_db_header __section(".data") = NULL; static inline const void *rsc_to_entry_header(const struct rsc_hdr *hdr) { u16 offset = le16_to_cpu(hdr->header_offset); @@ -123,24 +123,8 @@ rsc_offset(const struct rsc_hdr *hdr, const struct entry_header *ent) return cmd_db_header->data + offset + loffset; } -/** - * cmd_db_ready - Indicates if command DB is available - * - * Return: 0 on success, errno otherwise - */ -int cmd_db_ready(void) -{ - if (cmd_db_header == NULL) - return -EPROBE_DEFER; - else if (!cmd_db_magic_matches(cmd_db_header)) - return -EINVAL; - - return 0; -} -EXPORT_SYMBOL_GPL(cmd_db_ready); - static int cmd_db_get_header(const char *id, const struct entry_header **eh, const struct rsc_hdr **rh) { const struct rsc_hdr *rsc_hdr; @@ -194,55 +178,45 @@ u32 cmd_db_read_addr(const char *id) return ret < 0 ? 0 : le32_to_cpu(ent->addr); } EXPORT_SYMBOL_GPL(cmd_db_read_addr); -static int cmd_db_dev_probe(struct platform_device *pdev) +int cmd_db_bind(struct udevice *dev) { - struct reserved_mem *rmem; - int ret = 0; + void __iomem *base; + ofnode node; - rmem = of_reserved_mem_lookup(pdev->dev.of_node); - if (!rmem) { - dev_err(&pdev->dev, "failed to acquire memory region\n"); - return -EINVAL; - } - - cmd_db_header = memremap(rmem->base, rmem->size, MEMREMAP_WB); - if (!cmd_db_header) { - ret = -ENOMEM; - cmd_db_header = NULL; - return ret; + if (cmd_db_header) + return 0; + + node = dev_ofnode(dev); + + debug("%s(%s)\n", __func__, ofnode_get_name(node)); + + base = (void __iomem *)ofnode_get_addr(node); + if ((fdt_addr_t)base == FDT_ADDR_T_NONE) { + log_err("%s: Failed to read base address\n", __func__); + return -ENOENT; } + cmd_db_header = base; if (!cmd_db_magic_matches(cmd_db_header)) { - dev_err(&pdev->dev, "Invalid Command DB Magic\n"); + log_err("%s: Invalid Command DB Magic\n", __func__); return -EINVAL; } - device_set_pm_not_required(&pdev->dev); - return 0; } -static const struct of_device_id cmd_db_match_table[] = { +static const struct udevice_id cmd_db_ids[] = { { .compatible = "qcom,cmd-db" }, { } }; -MODULE_DEVICE_TABLE(of, cmd_db_match_table); -static struct platform_driver cmd_db_dev_driver = { - .probe = cmd_db_dev_probe, - .driver = { - .name = "cmd-db", - .of_match_table = cmd_db_match_table, - .suppress_bind_attrs = true, - }, +U_BOOT_DRIVER(qcom_cmd_db) = { + .name = "qcom_cmd_db", + .id = UCLASS_MISC, + .probe = cmd_db_bind, + .of_match = cmd_db_ids, }; -static int __init cmd_db_device_init(void) -{ - return platform_driver_register(&cmd_db_dev_driver); -} -core_initcall(cmd_db_device_init); - MODULE_DESCRIPTION("Qualcomm Technologies, Inc. Command DB Driver"); MODULE_LICENSE("GPL v2"); diff --git a/include/soc/qcom/cmd-db.h b/include/soc/qcom/cmd-db.h index 753c7923f8e5..1190f2c22cab 100644 --- a/include/soc/qcom/cmd-db.h +++ b/include/soc/qcom/cmd-db.h @@ -21,13 +21,10 @@ enum cmd_db_hw_type { #if IS_ENABLED(CONFIG_QCOM_COMMAND_DB) u32 cmd_db_read_addr(const char *resource_id); -int cmd_db_ready(void); #else static inline u32 cmd_db_read_addr(const char *resource_id) { return 0; } -static inline int cmd_db_ready(void) -{ return -ENODEV; } #endif /* CONFIG_QCOM_COMMAND_DB */ #endif /* __QCOM_COMMAND_DB_H__ */
Integrate cmd-db into the U-Boot driver model. This is just a wrapper around an in-memory database, so we just need to get the address and validate that cmd-db is there. Since cmd_db_header will be stored in the .data section we can skip bind if it's already set. Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org> --- To: Simon Glass <sjg@chromium.org> --- drivers/soc/qcom/cmd-db.c | 72 +++++++++++++++-------------------------------- include/soc/qcom/cmd-db.h | 3 -- 2 files changed, 23 insertions(+), 52 deletions(-)