Re: [PATCH v6 5/6] dt-bindings: mfd: Add Delta TN48M CPLD drivers bindings

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Robert,

sorry for slow reply, I am a bit busy.

On Tue, Aug 24, 2021 at 10:03 AM Robert Marko <robert.marko@xxxxxxxxxx> wrote:
> On Wed, Aug 11, 2021 at 2:17 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> >
> > On Tue, Aug 3, 2021 at 9:23 PM Robert Marko <robert.marko@xxxxxxxxxx> wrote:
> >
> > > The pins that this driver wants to expose are used for SFP-s only,
> > > they are provided by the Lattice CPLD which also does other things.
> > >
> > > Linux has a generic SFP driver which is used to manage these SFP
> > > ports, but it only supports GPIO-s, it has no concept of anything else.
> > > Since the driver is fully generic, I have no idea how could one extend it
> > > to effectively handle these pins internally, especially since I have more
> > > switches that use the CPLD for SFP-s as well, even for 48 ports and 192
> > > pins for them.
> >
> > Which file is this driver in so I can look?
>
> Hi Linus,
> Sorry for the late reply.
>
> Sure, here is the generic Linux driver that is used for SFP handling:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/phy/sfp.c?h=v5.14-rc7

So this has this:

enum {
        GPIO_MODDEF0,
        GPIO_LOS,
        GPIO_TX_FAULT,
        GPIO_TX_DISABLE,
        GPIO_RATE_SELECT,
        GPIO_MAX,

        SFP_F_PRESENT = BIT(GPIO_MODDEF0),
        SFP_F_LOS = BIT(GPIO_LOS),
        SFP_F_TX_FAULT = BIT(GPIO_TX_FAULT),
        SFP_F_TX_DISABLE = BIT(GPIO_TX_DISABLE),
        SFP_F_RATE_SELECT = BIT(GPIO_RATE_SELECT),

        SFP_E_INSERT = 0,
        SFP_E_REMOVE,

This does not look general purpose to me at all?
It's just some hardware engineer that thougt "GPIO"
was a nice thing to call this.

> > Maybe it is not a good idea to look for generic code just because
> > it is convenient? I have had this problem before with GPIO, along
> > the lines "lemme just do this dirty thing this one time because it
> > is so convenient for me" (more or less) and the answer is still
> > "no".
> >
> > Can you either augment the driver to handle a regmap with bit indices
> > instead or write a new similar driver for that or refactor it some other
> > way?
> >
> > It is not a simple solution to your problem, but it might be the right
> > solution even if it means some more work.
>
> I understand your position, believe me, I spend some time looking at
> what would be the logical way for these switches.
> But I see no way how could the SFP driver be extended in a generic way
> that would allow supporting different register layouts when it comes to pins.

Why do you think you have to use the GPIO abstraction and bindings?
Just invent something that satisfy your needs, the bindings are just
strings. Why does the consumer have to use the GPIO binding?
They can just use phandle named anything. Some "sfp-foo-resource = <&...>
or so.

For example I created this:
Documentation/devicetree/bindings/firmware/intel,ixp4xx-network-processing-engine.yaml
It's handling out a resource using a phandle. Nothing different than
GPIO, regulator, clock etc. Just invent something for SFP?

Yours,
Linus Walleij