| Message ID | d3c2333cfb681fd55390809d5d4527e6e5201bf3.1569591296.git.michal.simek@xilinx.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | arm64: zynqmp: Clean communication with PMUFW | expand |
Hi Ibai, Michal, On 27/09/19 15:34, Michal Simek wrote: > From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com> > > zynqmp-power driver for ZynqMP to handle the communication with the PMU > firmware. Firmware driver just probes subnodes and power driver handles > communication with PMU using the IPI mailbox driver. > > Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com> > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > --- > > drivers/firmware/Kconfig | 2 ++ > drivers/firmware/firmware-zynqmp.c | 40 ++++++++++++++++++++++++++++++ > 2 files changed, 42 insertions(+) > > diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig > index b70a2063551c..9596ec16c7f7 100644 > --- a/drivers/firmware/Kconfig > +++ b/drivers/firmware/Kconfig > @@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL > config ZYNQMP_FIRMWARE > bool "ZynqMP Firmware interface" > select FIRMWARE > + select ZYNQMP_IPI > + select DM_MAILBOX > help > Firmware interface driver is used by different > drivers to communicate with the firmware for > diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c > index 6644a7166ca0..b0930447b988 100644 > --- a/drivers/firmware/firmware-zynqmp.c > +++ b/drivers/firmware/firmware-zynqmp.c > @@ -1,6 +1,46 @@ > // SPDX-License-Identifier: GPL-2.0 > +/* > + * Xilinx Zynq MPSoC Firmware driver > + * > + * Copyright (C) 2018-2019 Xilinx, Inc. > + */ > > +#include <common.h> > #include <dm.h> > +#include <mailbox.h> > +#include <asm/arch/sys_proto.h> > + > +struct zynqmp_power { > + struct mbox_chan tx_chan; > + struct mbox_chan rx_chan; > +} zynqmp_power; > + > +static int zynqmp_power_probe(struct udevice *dev) > +{ > + int ret = 0; > + > + debug("%s, (dev=%p)\n", __func__, dev); > + > + ret |= mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan); > + ret |= mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan); If these two calls return different error values, the binary or will produce a nonsense 'ret' value. E.g. (-EINVAL | -ENODATA) equals -ENOTDIR. Otherwise looks good.
On 02. 10. 19 11:34, Luca Ceresoli wrote: > Hi Ibai, Michal, > > On 27/09/19 15:34, Michal Simek wrote: >> From: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com> >> >> zynqmp-power driver for ZynqMP to handle the communication with the PMU >> firmware. Firmware driver just probes subnodes and power driver handles >> communication with PMU using the IPI mailbox driver. >> >> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@xilinx.com> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> --- >> >> drivers/firmware/Kconfig | 2 ++ >> drivers/firmware/firmware-zynqmp.c | 40 ++++++++++++++++++++++++++++++ >> 2 files changed, 42 insertions(+) >> >> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig >> index b70a2063551c..9596ec16c7f7 100644 >> --- a/drivers/firmware/Kconfig >> +++ b/drivers/firmware/Kconfig >> @@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL >> config ZYNQMP_FIRMWARE >> bool "ZynqMP Firmware interface" >> select FIRMWARE >> + select ZYNQMP_IPI >> + select DM_MAILBOX >> help >> Firmware interface driver is used by different >> drivers to communicate with the firmware for >> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c >> index 6644a7166ca0..b0930447b988 100644 >> --- a/drivers/firmware/firmware-zynqmp.c >> +++ b/drivers/firmware/firmware-zynqmp.c >> @@ -1,6 +1,46 @@ >> // SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Xilinx Zynq MPSoC Firmware driver >> + * >> + * Copyright (C) 2018-2019 Xilinx, Inc. >> + */ >> >> +#include <common.h> >> #include <dm.h> >> +#include <mailbox.h> >> +#include <asm/arch/sys_proto.h> >> + >> +struct zynqmp_power { >> + struct mbox_chan tx_chan; >> + struct mbox_chan rx_chan; >> +} zynqmp_power; >> + >> +static int zynqmp_power_probe(struct udevice *dev) >> +{ >> + int ret = 0; >> + >> + debug("%s, (dev=%p)\n", __func__, dev); >> + >> + ret |= mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan); >> + ret |= mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan); > > If these two calls return different error values, the binary or will > produce a nonsense 'ret' value. E.g. (-EINVAL | -ENODATA) equals -ENOTDIR. > > Otherwise looks good. > Let's fix this. Also mbox_send/mbox_recv from firmware-zynqmp.c should be fixed too. Thanks, Michal
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index b70a2063551c..9596ec16c7f7 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -30,6 +30,8 @@ config TI_SCI_PROTOCOL config ZYNQMP_FIRMWARE bool "ZynqMP Firmware interface" select FIRMWARE + select ZYNQMP_IPI + select DM_MAILBOX help Firmware interface driver is used by different drivers to communicate with the firmware for diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 6644a7166ca0..b0930447b988 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -1,6 +1,46 @@ // SPDX-License-Identifier: GPL-2.0 +/* + * Xilinx Zynq MPSoC Firmware driver + * + * Copyright (C) 2018-2019 Xilinx, Inc. + */ +#include <common.h> #include <dm.h> +#include <mailbox.h> +#include <asm/arch/sys_proto.h> + +struct zynqmp_power { + struct mbox_chan tx_chan; + struct mbox_chan rx_chan; +} zynqmp_power; + +static int zynqmp_power_probe(struct udevice *dev) +{ + int ret = 0; + + debug("%s, (dev=%p)\n", __func__, dev); + + ret |= mbox_get_by_name(dev, "tx", &zynqmp_power.tx_chan); + ret |= mbox_get_by_name(dev, "rx", &zynqmp_power.rx_chan); + + if (ret) + debug("%s, cannot get mailboxes\n", __func__); + + return ret; +}; + +static const struct udevice_id zynqmp_power_ids[] = { + { .compatible = "xlnx,zynqmp-power" }, + { } +}; + +U_BOOT_DRIVER(zynqmp_power) = { + .name = "zynqmp_power", + .id = UCLASS_FIRMWARE, + .of_match = zynqmp_power_ids, + .probe = zynqmp_power_probe, +}; static const struct udevice_id zynqmp_firmware_ids[] = { { .compatible = "xlnx,zynqmp-firmware" },