Re: [RFC v1] mfd: pinctrl: RFC only: add and utilze mfd option in pinctrl-ocelot

From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Colin Foster <colin.foster@in-advantage.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
	Lee Jones <lee.jones@linaro.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	"katie.morris@in-advantage.com" <katie.morris@in-advantage.com>
Subject: Re: [RFC v1] mfd: pinctrl: RFC only: add and utilze mfd option in pinctrl-ocelot
Date: Mon, 6 Dec 2021 17:04:34 +0000	[thread overview]
Message-ID: <20211206170433.rwvt3t2rllj5xlw2@skbuf> (raw)
In-Reply-To: <20211206000311.GA1094021@euler>

On Sun, Dec 05, 2021 at 04:03:11PM -0800, Colin Foster wrote:
> I've started venturing down this path, and am already hitting a couple
> bumps... and they're bumps I've hit in the existing driver as well.
> Basically I couldn't use "ocelot_reg_write" before calling
> "dsa_register_switch". That's now a bigger issue with MFD.

By the way, it turns out that the comment above felix_setup() is bogus -
I didn't know at the time what the issue was, and it was solved by
Claudiu through commit b4024c9e5c57 ("felix: Fix initialization of
ioremap resources"). If it helps to move the ocelot_regmap_init() call
to the probe path, sure you can do that now.

> So the first thing I'll probably want to do in drivers/mfd/ocelot-spi
> is reset the device. The current implementation of this uses
> ocelot_field_write with GCB_SOFT_RST_CHIP_RST, and some SYS registers as
> well... I don't think those registers will be needed elsewhere, so can
> be defined and limited to ocelot-mfd-core.
>
> As I'm writing this though... that seems like it might be a good thing.
> ocelot_switch doesn't need to know about reset registers necessarily. If
> there are cases where register addresses need to be shared I'll cross
> that bridge when I get to it... but maybe I'll get lucky.
>
> (Sorry - I'm thinking out loud)

According to my documentation, DEVCPU_GCB triggers a chip-wide soft
reset, and that may affect more IP blocks than just the switching core.
On the other hand, the switching core is all that the NXP parts
integrate, so I wouldn't be able to tell you more than that...
I think it would make sense for you to split the reset sequence into a
part (for DEVCPU_GCB) that is done in the top-level mfd driver, and more
fine-grained ones in places such as your own ocelot->ops->reset()
implementation. Anyway, as mentioned above, this is orthogonal to the
regmap issue. I don't know why I realized just now that this is what
your problem was, sorry.