Re: [PATCH RESEND 3/6] pci: add host controller struct to sysdata

[Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>]

On Fri, Jul 25, 2014 at 11:07 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Mittwoch, den 23.07.2014, 11:26 +0200 schrieb Sebastian Hesselbarth:
>> struct pci_bus allows to set some private sysdata. Assign the host
>> controller struct to it by default.
>>
>> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> ---
>> To: barebox@lists.infradead.org
>> To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> Cc: Antony Pavlov <antonynpavlov@gmail.com>
>> Cc: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>> Cc: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
>> ---
>>  drivers/pci/pci.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index e5cd8a33b2be..b30cdd96f8ca 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -39,6 +39,7 @@ void register_pci_controller(struct pci_controller *hose)
>>       bus->ops = hose->pci_ops;
>>       bus->resource[0] = hose->mem_resource;
>>       bus->resource[1] = hose->io_resource;
>> +     bus->sysdata = hose;
>>
> This doesn't seem right. You are not only assigning hose to sysdata by
> default, but make it the only option as there is no sane way to override
> this assignment before bus scanning happens.

Yeah, the is some things with pci core already, that make it seem not right,
e.g. IIRC it is pci_register_driver which makes it impossible to use the
register_driver_macro() as it expects <bus>_driver_register.

My concern still stands, that it may be more useful to split above up in
_alloc and _register phase.

> Either make this a parameter to this function, so pci host controller
> drivers can pass in whatever they want as the sysdata, or add an
> explicit backlink pointer from bus to controller. Hm, maybe even both.

I guess it will be best to have both, i.e. bus->host = hose and an extra
parameter to pass sysdata. Or we could also drop sysdata now until
anyone requires it.

Sebastian

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox