| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 28 Sep 2022 00:04:11 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: Colin Foster <colin.foster@...advantage.com>
Cc: linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, netdev@...r.kernel.org,
Russell King <linux@...linux.org.uk>,
Linus Walleij <linus.walleij@...aro.org>,
UNGLinuxDriver@...rochip.com,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Claudiu Manoil <claudiu.manoil@....com>,
Lee Jones <lee@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>,
"David S. Miller" <davem@...emloft.net>,
Florian Fainelli <f.fainelli@...il.com>,
Vivien Didelot <vivien.didelot@...il.com>,
Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH v3 net-next 11/14] mfd: ocelot: add regmaps for ocelot_ext
On Sun, Sep 25, 2022 at 05:29:25PM -0700, Colin Foster wrote:
> The Ocelot switch core driver relies heavily on a fixed array of resources
> for both ports and peripherals. This is in contrast to existing peripherals
> - pinctrl for example - which have a one-to-one mapping of driver <>
> resource. As such, these regmaps must be created differently so that
> enumeration-based offsets are preserved.
>
> Register the regmaps to the core MFD device unconditionally so they can be
> referenced by the Ocelot switch / Felix DSA systems.
>
> Signed-off-by: Colin Foster <colin.foster@...advantage.com>
> ---
>
> v3
> * No change
>
> v2
> * Alignment of variables broken out to a separate patch
> * Structs now correctly use EXPORT_SYMBOL*
> * Logic moved and comments added to clear up conditionals around
> vsc7512_target_io_res[i].start
>
> v1 from previous RFC:
> * New patch
>
> ---
> drivers/mfd/ocelot-core.c | 87 ++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/ocelot.h | 5 +++
> 2 files changed, 92 insertions(+)
>
> diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c
> index 013e83173062..702555fbdcc5 100644
> --- a/drivers/mfd/ocelot-core.c
> +++ b/drivers/mfd/ocelot-core.c
> @@ -45,6 +45,45 @@
> #define VSC7512_SIO_CTRL_RES_START 0x710700f8
> #define VSC7512_SIO_CTRL_RES_SIZE 0x00000100
>
> +#define VSC7512_HSIO_RES_START 0x710d0000
> +#define VSC7512_HSIO_RES_SIZE 0x00000128
I don't think you should give the HSIO resource to the switching driver.
In drivers/net/ethernet/mscc/ocelot_vsc7514.c, there is this comment:
static void ocelot_pll5_init(struct ocelot *ocelot)
{
/* Configure PLL5. This will need a proper CCF driver
* The values are coming from the VTSS API for Ocelot
*/
I believe CCF stands for Common Clock Framework.
> +
> +#define VSC7512_ANA_RES_START 0x71880000
> +#define VSC7512_ANA_RES_SIZE 0x00010000
> +
> +#define VSC7512_QS_RES_START 0x71080000
> +#define VSC7512_QS_RES_SIZE 0x00000100
> +
> +#define VSC7512_QSYS_RES_START 0x71800000
> +#define VSC7512_QSYS_RES_SIZE 0x00200000
> +
> +#define VSC7512_REW_RES_START 0x71030000
> +#define VSC7512_REW_RES_SIZE 0x00010000
> +
> +#define VSC7512_SYS_RES_START 0x71010000
> +#define VSC7512_SYS_RES_SIZE 0x00010000
> +
> +#define VSC7512_S0_RES_START 0x71040000
> +#define VSC7512_S1_RES_START 0x71050000
> +#define VSC7512_S2_RES_START 0x71060000
> +#define VSC7512_S_RES_SIZE 0x00000400
VCAP_RES_SIZE?
> +
> +#define VSC7512_GCB_RES_START 0x71070000
> +#define VSC7512_GCB_RES_SIZE 0x0000022c
Again, I don't think devcpu_gcb should be given to a switching-only
driver. There's nothing switching-related about it.
> +#define VSC7512_PORT_0_RES_START 0x711e0000
> +#define VSC7512_PORT_1_RES_START 0x711f0000
> +#define VSC7512_PORT_2_RES_START 0x71200000
> +#define VSC7512_PORT_3_RES_START 0x71210000
> +#define VSC7512_PORT_4_RES_START 0x71220000
> +#define VSC7512_PORT_5_RES_START 0x71230000
> +#define VSC7512_PORT_6_RES_START 0x71240000
> +#define VSC7512_PORT_7_RES_START 0x71250000
> +#define VSC7512_PORT_8_RES_START 0x71260000
> +#define VSC7512_PORT_9_RES_START 0x71270000
> +#define VSC7512_PORT_10_RES_START 0x71280000
> +#define VSC7512_PORT_RES_SIZE 0x00010000
> +
> #define VSC7512_GCB_RST_SLEEP_US 100
> #define VSC7512_GCB_RST_TIMEOUT_US 100000
>
> @@ -96,6 +135,36 @@ static const struct resource vsc7512_sgpio_resources[] = {
> DEFINE_RES_REG_NAMED(VSC7512_SIO_CTRL_RES_START, VSC7512_SIO_CTRL_RES_SIZE, "gcb_sio"),
> };
>
> +const struct resource vsc7512_target_io_res[TARGET_MAX] = {
> + [ANA] = DEFINE_RES_REG_NAMED(VSC7512_ANA_RES_START, VSC7512_ANA_RES_SIZE, "ana"),
> + [QS] = DEFINE_RES_REG_NAMED(VSC7512_QS_RES_START, VSC7512_QS_RES_SIZE, "qs"),
> + [QSYS] = DEFINE_RES_REG_NAMED(VSC7512_QSYS_RES_START, VSC7512_QSYS_RES_SIZE, "qsys"),
> + [REW] = DEFINE_RES_REG_NAMED(VSC7512_REW_RES_START, VSC7512_REW_RES_SIZE, "rew"),
> + [SYS] = DEFINE_RES_REG_NAMED(VSC7512_SYS_RES_START, VSC7512_SYS_RES_SIZE, "sys"),
> + [S0] = DEFINE_RES_REG_NAMED(VSC7512_S0_RES_START, VSC7512_S_RES_SIZE, "s0"),
> + [S1] = DEFINE_RES_REG_NAMED(VSC7512_S1_RES_START, VSC7512_S_RES_SIZE, "s1"),
> + [S2] = DEFINE_RES_REG_NAMED(VSC7512_S2_RES_START, VSC7512_S_RES_SIZE, "s2"),
> + [GCB] = DEFINE_RES_REG_NAMED(VSC7512_GCB_RES_START, VSC7512_GCB_RES_SIZE, "devcpu_gcb"),
> + [HSIO] = DEFINE_RES_REG_NAMED(VSC7512_HSIO_RES_START, VSC7512_HSIO_RES_SIZE, "hsio"),
> +};
> +EXPORT_SYMBOL_NS(vsc7512_target_io_res, MFD_OCELOT);
> +
> +const struct resource vsc7512_port_io_res[] = {
I hope you will merge these 2 arrays now.
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_0_RES_START, VSC7512_PORT_RES_SIZE, "port0"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_1_RES_START, VSC7512_PORT_RES_SIZE, "port1"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_2_RES_START, VSC7512_PORT_RES_SIZE, "port2"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_3_RES_START, VSC7512_PORT_RES_SIZE, "port3"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_4_RES_START, VSC7512_PORT_RES_SIZE, "port4"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_5_RES_START, VSC7512_PORT_RES_SIZE, "port5"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_6_RES_START, VSC7512_PORT_RES_SIZE, "port6"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_7_RES_START, VSC7512_PORT_RES_SIZE, "port7"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_8_RES_START, VSC7512_PORT_RES_SIZE, "port8"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_9_RES_START, VSC7512_PORT_RES_SIZE, "port9"),
> + DEFINE_RES_REG_NAMED(VSC7512_PORT_10_RES_START, VSC7512_PORT_RES_SIZE, "port10"),
> + {}
> +};
> +EXPORT_SYMBOL_NS(vsc7512_port_io_res, MFD_OCELOT);
> +
> static const struct mfd_cell vsc7512_devs[] = {
> {
> .name = "ocelot-pinctrl",
> @@ -144,6 +213,7 @@ static void ocelot_core_try_add_regmaps(struct device *dev,
>
> int ocelot_core_init(struct device *dev)
> {
> + const struct resource *port_res;
> int i, ndevs;
>
> ndevs = ARRAY_SIZE(vsc7512_devs);
> @@ -151,6 +221,23 @@ int ocelot_core_init(struct device *dev)
> for (i = 0; i < ndevs; i++)
> ocelot_core_try_add_regmaps(dev, &vsc7512_devs[i]);
>
> + /*
> + * Both the target_io_res and the port_io_res structs need to be referenced directly by
> + * the ocelot_ext driver, so they can't be attached to the dev directly and referenced by
> + * offset like the rest of the drivers. Instead, create these regmaps always and allow any
> + * children look these up by name.
> + */
> + for (i = 0; i < TARGET_MAX; i++)
> + /*
> + * The target_io_res array is sparsely populated. Use .start as an indication that
> + * the entry isn't defined
> + */
> + if (vsc7512_target_io_res[i].start)
> + ocelot_core_try_add_regmap(dev, &vsc7512_target_io_res[i]);
> +
> + for (port_res = vsc7512_port_io_res; port_res->start; port_res++)
> + ocelot_core_try_add_regmap(dev, port_res);
> +
Will need to be updated.
> return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO, vsc7512_devs, ndevs, NULL, 0, NULL);
> }
> EXPORT_SYMBOL_NS(ocelot_core_init, MFD_OCELOT);
> diff --git a/include/linux/mfd/ocelot.h b/include/linux/mfd/ocelot.h
> index dd72073d2d4f..439ff5256cf0 100644
> --- a/include/linux/mfd/ocelot.h
> +++ b/include/linux/mfd/ocelot.h
> @@ -11,8 +11,13 @@
> #include <linux/regmap.h>
> #include <linux/types.h>
>
> +#include <soc/mscc/ocelot.h>
> +
Is this the problematic include that makes it necessary to have the
pinctrl hack? Can we drop the #undef REG now?
> struct resource;
>
> +extern const struct resource vsc7512_target_io_res[TARGET_MAX];
> +extern const struct resource vsc7512_port_io_res[];
> +
Will need to be removed.
> static inline struct regmap *
> ocelot_regmap_from_resource_optional(struct platform_device *pdev,
> unsigned int index,
> --
> 2.25.1
>
Powered by blists - more mailing lists