Re: [PATCH] serial: 8250: Add support for using platform_device resources

[Esben Haabendal]

Andy Shevchenko <andy.shevchenko@xxxxxxxxx> writes:

> On Tue, May 14, 2019 at 10:24 AM Esben Haabendal <esben@xxxxxxxxxxxx> wrote:
>> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> writes:
>> > On Tue, May 07, 2019 at 02:22:18PM +0200, Esben Haabendal wrote:
>
>> We are on repeat here.  I don't agree with you here.  I have a simple
>> generic 8250 (16550A) compatible device, and cannot use it in a mfd
>> driver using the standard mfd-core framework.
>
>> The lacking of support for platform_get_resource() in the generic
>> serial8250 driver is not a feature.  It should be supported, just as it
>> is in several of the specialized 8250 drivers.
>
> We are going circles here.
> What exactly prevents you to use it? Presence of request_mem_region()?

Exactly.

>> It would still mean that I would have revert to not using convenient and
>> otherwise fully appropriate API calls like pci_request_regions() and
>> mfd_add_devices().
>
> Yes, here is the issue. 8250 requires the parent not to *request*
> resources. Because child handles IO access itself.

Ok, clearly we are not discussing the actual IO access.  The only issue
is how to handle the memory resource management.

And yes, serial8250 requires "the parent" to not request the resources.
But by doing so, it gets in the way of the mfd-core way of splitting a
properly requested resource.

>> The mfd driver in question is for a PCI device.  Not being able to
>> request the PCI regions seems silly.
>
> Nope. Otherwise, the parent which *doesn't handle* IO on behalf of
> child should not request its resources.

If I may, could I get you to discuss this with Lee Jones?

As I read both of your comments in this thread, you are not aligned on
how mfd drivers should handle resources.  And in that case, one of you
are most likely more right than the other, and if Lee is right, I seem
to be unable to convince you about that.

>> Not being able to register all child devices with the call introduced
>> for that sole purpose also seems silly.
>
>> Please take a look at https://lkml.org/lkml/2019/4/9/576
>> ("[PATCH v2 2/4] mfd: ioc3: Add driver for SGI IOC3 chip")
>
> Thank you for this link.
> Now, look at this comment:
>
> + /*
> + * Map all IOC3 registers.  These are shared between subdevices
> + * so the main IOC3 module manages them.
> + */
>
> Is it your case? Can we see the code?

That comment seems quite misleading.  I am quote certain that the uart
registers which are part of BAR 0 is not more shared between subdevices
than the uart registers in BAR 0 in my case.

But BAR 0 as a whole is shared between subdevices.  But BAR 0 can be
(is) split in parts that are exclusive to one subdevice.  The only
difference I see is that I don't have any registers accessed directly by
the mfd driver.

/Esben