Source code organization

[Greg KH]

On Fri, Oct 21, 2022 at 01:51:48AM +0000, Billie Alsup (balsup) wrote:
> 
> > From: Greg KH <greg at kroah.com>
> >        - is this something else?  Then pick a place and submit a patch
> >          and people will tell you if you got it wrong :)
> 
> I think this is going to be my strategy, except per a separate recommendation,
> I will put it in drivers/platform/cisco, similar to the existing chrome, goldfish,
> mellanox, and surface already under drivers/platform.  These drivers really
> are Cisco hardware specific, so I'd like to keep them grouped together,
> if reviewers will allow it!

Platform drivers are the "catch all" for where you have to talk to tiny
hardware-specific devices.  That's normally not an i2c controller.

> > But step back, why are you creating MFD devices anyway?  Why not make
> > "real" devices in your hardware representation as you control the FPGA
> > and DT definitions, right?
> 
> Currently the FPGAs themselves are discovered (for pci bus), or explicitly
> instantiated (for i2c, whether via user mode script, device tree, or
> ACPI configuration).

So you are creating platform devices for a dynamic device that is on
another bus?  Please no, that is an abuse of the platform device code.
That's the biggest reason NOT to use MFD for your code.

Just make normal drivers for these, no need for MFD at all, why do you
want to use that api here?

> The slpc and p2pm MFD drivers also "discover"
> the peer FPGA. Are you suggesting that I create a new bus driver
> for a Cisco FPGA?

If it is a bus, then yes, you should.  That's a very simple thing to do.

> I was under the impression that the MFD framework was explicitly meant
> for this type of application.

Nope!  But it has been abused for situations like this.  there are
simpler ways to do this now, including the auxiliary bus code, which
might be what you need to use here instead.

> I do dynamically
> create the array of struct mfd_cell passed to devm_mfd_add_devices
> (versus having an fpga specific driver), which achieves the same 
> purpose as a bus driver (I think).

Close, but you are making fake platform devices for a dynamic discovered
device, which is not ok.  Please use a real bus for this, either your
own, or the aux bus code.

> It's just that I thought MFD framework
> was meant for this type of application, and that's what I went with.
> The drivers themselves have been written over the last several years
> (starting with use in ONIE and SONiC, and now being used with FBOSS).  It
> is only now that I have been authorized to make the source public.  Changing
> to a bus driver top layer would seem to be a very big change.  There
> are 30 or so drivers being used today with Cisco FPGAs, and
> that number will only grow over time.

Changing the bus layer should not be a big deal, and just because the
code is old doesn't mean it has to be fixed up properly to get it merged
into the tree.  That's the best reason for why it needs to be fixed up,
you shouldn't be using apis and interfaces that are not designed for
what you are doing :)

Do you have a pointer to the source for these?  That might make it
easier to discuss than general concepts here.

thanks,

greg k-h