Hello Lee, The latest revision [0] has changed approach following advice from Stephen Boyd to rather move forward with auxiliary devices. [0]: https://lore.kernel.org/lkml/20240620-mbly-olb-v3-0-5f29f8ca289c@xxxxxxxxxxx/ On Fri May 31, 2024 at 1:05 PM CEST, Lee Jones wrote: > On Fri, 03 May 2024, Théo Lebrun wrote: > > > Mobileye OLB system-controller gets used in EyeQ5, EyeQ6L and EyeQ6H > > platforms. It hosts clock, reset and pinctrl functionality. > > > > Tiny iomem resources are declared for all cells. Some features are > > spread apart. Pinctrl is only used on EyeQ5. > > > > EyeQ6H is special: it hosts seven OLB controllers, each with a > > compatible. That means many clock and reset cells. Use cell->devname > > for explicit device names rather than clk-eyeq.ID or clk-eyeq.ID.auto. > > > > Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx> > > --- > > drivers/mfd/Kconfig | 10 +++ > > drivers/mfd/Makefile | 2 + > > drivers/mfd/mobileye-olb.c | 180 +++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 192 insertions(+) > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index 4b023ee229cf..d004a3f4d493 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -1030,6 +1030,16 @@ config MFD_OCELOT > > > > If unsure, say N. > > > > +config MFD_OLB > > + bool "Mobileye EyeQ OLB System Controller Support" > > + select MFD_CORE > > + depends on MACH_EYEQ5 || MACH_EYEQ6H || COMPILE_TEST > > + default MACH_EYEQ5 || MACH_EYEQ6H > > + help > > + Say yes here to add support for EyeQ platforms (EyeQ5, EyeQ6L and > > + EyeQ6H). This core MFD platform driver provides clock, reset and > > + pinctrl (only EyeQ5) support. > > + > > config EZX_PCAP > > bool "Motorola EZXPCAP Support" > > depends on SPI_MASTER > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index c66f07edcd0e..d872833966a8 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -120,6 +120,8 @@ obj-$(CONFIG_MFD_CORE) += mfd-core.o > > ocelot-soc-objs := ocelot-core.o ocelot-spi.o > > obj-$(CONFIG_MFD_OCELOT) += ocelot-soc.o > > > > +obj-$(CONFIG_MFD_OLB) += mobileye-olb.o > > + > > obj-$(CONFIG_EZX_PCAP) += ezx-pcap.o > > obj-$(CONFIG_MFD_CPCAP) += motorola-cpcap.o > > > > diff --git a/drivers/mfd/mobileye-olb.c b/drivers/mfd/mobileye-olb.c > > new file mode 100644 > > index 000000000000..1640d63a3ddd > > --- /dev/null > > +++ b/drivers/mfd/mobileye-olb.c > > @@ -0,0 +1,180 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * System controller multi-function device for EyeQ platforms. > > + * > > + * Mobileye EyeQ5, EyeQ6L and EyeQ6H platforms have MMIO mapped registers > > + * controlling core platform clocks, resets and pin control. Many other > > + * features are present and not yet exposed. > > + * > > + * Declare cells for each compatible. Only EyeQ5 has pinctrl. > > + * EyeQ6H has seven OLB instances; each has a name which we propagate to > > + * sub-devices using cell->devname. > > + * > > + * Copyright (C) 2024 Mobileye Vision Technologies Ltd. > > + */ > > + > > +#include <linux/array_size.h> > > +#include <linux/device.h> > > +#include <linux/errno.h> > > +#include <linux/ioport.h> > > +#include <linux/mfd/core.h> > > +#include <linux/mod_devicetable.h> > > +#include <linux/platform_device.h> > > +#include <linux/property.h> > > + > > +#define OLB_MFD_CELL(_name, _res, _devname) \ > > + MFD_CELL_ALL(_name, _res, NULL, 0, 0, NULL, 0, false, NULL, _devname) > > The reason we provide generic MACROs is so that you don't have to define > your own. In this case, there was no macro that allowed setting the MFD cell devname field, which is something that was added in the series. I didn't know what generic macro to create so instead of creating loads of useless ones I created a single one, targeted at my usecase. > > > +struct olb_match_data { > > + const struct mfd_cell *cells; > > + int nb_cells; /* int to match devm_mfd_add_devices() argument */ > > +}; > > + > > +#define OLB_DATA(_cells) { .cells = (_cells), .nb_cells = ARRAY_SIZE(_cells) } > > + > > +static int olb_probe(struct platform_device *pdev) > > +{ > > + const struct olb_match_data *match_data; > > + struct device *dev = &pdev->dev; > > + struct resource *res; > > + > > + match_data = device_get_match_data(dev); > > + if (!match_data) > > + return -ENODEV; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + if (!res) > > + return -ENODEV; > > + > > + return devm_mfd_add_devices(dev, PLATFORM_DEVID_NONE, > > + match_data->cells, match_data->nb_cells, > > + res, 0, NULL); > > +} > > + > > +static const struct resource olb_eyeq5_clk_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x02C, 10 * 8, "pll"), > > + DEFINE_RES_MEM_NAMED(0x11C, 1 * 4, "ospi"), > > +}; > > + > > +static const struct resource olb_eyeq5_reset_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x004, 2 * 4, "d0"), > > + DEFINE_RES_MEM_NAMED(0x200, 13 * 4, "d1"), > > + DEFINE_RES_MEM_NAMED(0x120, 1 * 4, "d2"), > > +}; > > + > > +static const struct resource olb_eyeq5_pinctrl_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x0B0, 12 * 4, "pinctrl"), > > +}; > > + > > +static const struct mfd_cell olb_eyeq5_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq5_clk_resources, NULL), > > + OLB_MFD_CELL("reset-eyeq", olb_eyeq5_reset_resources, NULL), > > + OLB_MFD_CELL("eyeq5-pinctrl", olb_eyeq5_pinctrl_resources, NULL), > > +}; > > + > > +static const struct olb_match_data olb_eyeq5_match_data = OLB_DATA(olb_eyeq5_cells); > > + > > +static const struct resource olb_eyeq6l_clk_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x02C, 4 * 8, "pll"), > > +}; > > + > > +static const struct resource olb_eyeq6l_reset_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x004, 2 * 4, "d0"), > > + DEFINE_RES_MEM_NAMED(0x200, 13 * 4, "d1"), > > +}; > > + > > +static const struct mfd_cell olb_eyeq6l_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq6l_clk_resources, NULL), > > + OLB_MFD_CELL("reset-eyeq", olb_eyeq6l_reset_resources, NULL), > > +}; > > + > > +static const struct olb_match_data olb_eyeq6l_match_data = OLB_DATA(olb_eyeq6l_cells); > > + > > +static const struct resource olb_eyeq6h_acc_clk_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x040, 7 * 8, "pll"), > > +}; > > + > > +static const struct resource olb_eyeq6h_acc_reset_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x000, 15 * 4, "d0"), > > +}; > > + > > +static const struct mfd_cell olb_eyeq6h_acc_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq6h_acc_clk_resources, "clk-eyeq-acc"), > > The point of enumerating platform device names is to identify devices > that are identical. We lose this with bespoke naming. > > If you want to identify devices either define a value to pass to .id or > adapt the first parameter and make the clk-eyeq driver accept different > device names. About int IDs: those make it unclear what device we are talking about, when we have seven of them as it was the case in this series. Yes, there were 7 system-controllers, each exposing clocks (and most exposing resets). IDs from 0 thru 6 was really not clear enough when going through dmesg, which I why I added the feature. I wasn't able to change the first parameter as that gets used for matching a driver. As it is driver name, there can only be a single string per driver. Here we have multiple MFD cells that each want to probe the same driver, so they must have the same name. > > > + OLB_MFD_CELL("reset-eyeq", olb_eyeq6h_acc_reset_resources, "reset-eyeq-acc"), > > +}; > > + > > +static const struct olb_match_data olb_eyeq6h_acc_match_data = OLB_DATA(olb_eyeq6h_acc_cells); > > + > > +static const struct resource olb_eyeq6h_we_clk_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x074, 1 * 8, "pll"), > > +}; > > + > > +static const struct resource olb_eyeq6h_we_reset_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x004, 4 * 4, "d0"), > > +}; > > + > > +static const struct mfd_cell olb_eyeq6h_west_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq6h_we_clk_resources, "clk-eyeq-west"), > > + OLB_MFD_CELL("reset-eyeq", olb_eyeq6h_we_reset_resources, "reset-eyeq-west"), > > +}; > > + > > +static const struct olb_match_data olb_eyeq6h_west_match_data = OLB_DATA(olb_eyeq6h_west_cells); > > + > > +static const struct mfd_cell olb_eyeq6h_east_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq6h_we_clk_resources, "clk-eyeq-east"), > > + OLB_MFD_CELL("reset-eyeq", olb_eyeq6h_we_reset_resources, "reset-eyeq-east"), > > +}; > > + > > +static const struct olb_match_data olb_eyeq6h_east_match_data = OLB_DATA(olb_eyeq6h_east_cells); > > + > > +static const struct resource olb_eyeq6h_south_clk_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x000, 4 * 8, "pll"), > > + DEFINE_RES_MEM_NAMED(0x070, 1 * 4, "emmc"), > > + DEFINE_RES_MEM_NAMED(0x090, 1 * 4, "ospi"), > > + DEFINE_RES_MEM_NAMED(0x098, 1 * 4, "tsu"), > > +}; > > + > > +static const struct mfd_cell olb_eyeq6h_south_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq6h_south_clk_resources, "clk-eyeq-south"), > > +}; > > + > > +static const struct olb_match_data olb_eyeq6h_south_match_data = OLB_DATA(olb_eyeq6h_south_cells); > > + > > +static const struct resource olb_eyeq6h_ddr_clk_resources[] = { > > + DEFINE_RES_MEM_NAMED(0x074, 1 * 8, "pll"), > > +}; > > + > > +static const struct mfd_cell olb_eyeq6h_ddr0_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq6h_ddr_clk_resources, "clk-eyeq-ddr0"), > > +}; > > + > > +static const struct olb_match_data olb_eyeq6h_ddr0_match_data = OLB_DATA(olb_eyeq6h_ddr0_cells); > > + > > +static const struct mfd_cell olb_eyeq6h_ddr1_cells[] = { > > + OLB_MFD_CELL("clk-eyeq", olb_eyeq6h_ddr_clk_resources, "clk-eyeq-ddr1"), > > +}; > > + > > +static const struct olb_match_data olb_eyeq6h_ddr1_match_data = OLB_DATA(olb_eyeq6h_ddr1_cells); > > + > > +static const struct of_device_id olb_of_match[] = { > > + { .compatible = "mobileye,eyeq5-olb", .data = &olb_eyeq5_match_data }, > > We're not passing MFD init data through the OF API, sorry. > > Pass defined identifiers through instead and match on those please. Ah, that makes sense. Thanks for the mention. I can't find any reasoning, I might be missing context? Thanks Lee, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
