On 09/13/2014 09:46 PM, Grant Likely wrote:
> On Mon, 08 Sep 2014 13:22:44 -0700, Stephen Boyd <[email protected]> wrote:
>>
>> Where is this described? From the commit text that introduces
>> IORESOURCE_REG I see:
>>
>> "Currently a bunch of I2C/SPI MFD drivers are using IORESOURCE_IO for
>> register address ranges. Since this causes some confusion due to the
>> primary use of this resource type for PCI/ISA I/O ports create a new
>> resource type IORESOURCE_REG."
> Sorry, I mistook IORESOURCE_REG or IORESOURCE_IO. You're right, this
> isn't an issue.
>
> I'm still concerned about the implications of automatically populating
> platform_devices with this resource type. I'll talk to Mark about it
> face to fact at Connect this week.
>
>
Where did this end up? When we talked at Connect I think we settled on
exploring a driver core specific API like dev_get_localbus_address()
that calls of_get_localbus_address() for devices with an of_node and in
the future it could call something like acpi_get_localbus_address() when
there's an acpi_node. I believe the biggest concern is that we're making
an API that is OF or platform bus specific when it doesn't need to be.
Making a driver core specific API avoids this problem by making it bus
agnostic.
That's fine, but I still think we want to have the IORESOURCE_REG
resources given that we have drivers in-flight and some already merged
that are using the IORESOURCE_REG resource. Furthermore, ACPI is using
the platform bus for MFDs so it's not like we're going to be using a
different bus in the future for these pmic sub-device drivers if we
decide to do pmic devices in ACPI (looks like there is at least
precedence for that with Intel's i2c pmic). Supporting this on ACPI is
going to take the same effort if we plumb it into the resource table or
we plumb it into a new driver core API, so the bus agnostic angle isn't
looking all that convincing. Not to say I'm opposed to some driver core
specific API (that's similar to the proposed device_property_read_*()
API) but I don't see any benefit for something that is already "unified"
between ACPI and OF via the platform bus resources table. If the
resources table didn't already exist I'd be more inclined to say we
should use some new driver core API.
So what's the way forward? I see a few options:
1) Take this patch after some minor tweaks
2) Add a driver core API and fixup all drivers to use it
3) Layer the platform resource stuff on top of a driver core API
I'd prefer we went with option #1.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Wed, Oct 22, 2014 at 04:01:26PM -0700, Stephen Boyd wrote:
> Where did this end up? When we talked at Connect I think we settled on
> exploring a driver core specific API like dev_get_localbus_address()
> that calls of_get_localbus_address() for devices with an of_node and in
> the future it could call something like acpi_get_localbus_address() when
> there's an acpi_node. I believe the biggest concern is that we're making
> an API that is OF or platform bus specific when it doesn't need to be.
> Making a driver core specific API avoids this problem by making it bus
> agnostic.
Given how little information there is in the original patch as to exactly
what problem this is addressing, I could be getting the wrong end of the
stick here.
Is this about trying to have a way to obtain the bus local addresses
associated with CPU-view resources?
If so, how about looking towards PCI, which has had this problem for the
last 15+ years, where PCI bus addresses are not necessarily the same as
CPU physical addresses?
There, we don't end up with multiple addresses specified in resources.
We instead have a way to translate between resources and bus-local
addresses, which IMHO is far nicer and less error-prone than having to
specify the same information twice, once with an offset and once without.
--
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
On Wed, Oct 22, 2014 at 04:01:26PM -0700, Stephen Boyd wrote:
> >> "Currently a bunch of I2C/SPI MFD drivers are using IORESOURCE_IO for
> >> register address ranges. Since this causes some confusion due to the
> >> primary use of this resource type for PCI/ISA I/O ports create a new
> >> resource type IORESOURCE_REG."
> > Sorry, I mistook IORESOURCE_REG or IORESOURCE_IO. You're right, this
> > isn't an issue.
> > I'm still concerned about the implications of automatically populating
> > platform_devices with this resource type. I'll talk to Mark about it
> > face to fact at Connect this week.
Hrm, missed that discussion.
> Where did this end up? When we talked at Connect I think we settled on
> exploring a driver core specific API like dev_get_localbus_address()
> that calls of_get_localbus_address() for devices with an of_node and in
...
> That's fine, but I still think we want to have the IORESOURCE_REG
> resources given that we have drivers in-flight and some already merged
> that are using the IORESOURCE_REG resource. Furthermore, ACPI is using
Right, IORESOURCE_REG was the original solution to the overlapping use
of IORESOURCE_IO (rather than having multiple IORESOURCE_IO trees since
we do special magic for IORESOURCE_IO for legacy reasons which was
causing issues but the idea of doing it for generic I/O make sense).
> the platform bus for MFDs so it's not like we're going to be using a
> different bus in the future for these pmic sub-device drivers if we
> decide to do pmic devices in ACPI (looks like there is at least
> precedence for that with Intel's i2c pmic). Supporting this on ACPI is
> going to take the same effort if we plumb it into the resource table or
> we plumb it into a new driver core API, so the bus agnostic angle isn't
Even if we do these things in ACPI it's not at all clear to me that the
idea of putting the internals of the device in ACPI would be at all
tasteful there. Personally I'm not a big fan of always doing it in DT
either but it's more tasteful there and definitely does make sense for
some things.
On 10/22/2014 04:20 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 22, 2014 at 04:01:26PM -0700, Stephen Boyd wrote:
>> Where did this end up? When we talked at Connect I think we settled on
>> exploring a driver core specific API like dev_get_localbus_address()
>> that calls of_get_localbus_address() for devices with an of_node and in
>> the future it could call something like acpi_get_localbus_address() when
>> there's an acpi_node. I believe the biggest concern is that we're making
>> an API that is OF or platform bus specific when it doesn't need to be.
>> Making a driver core specific API avoids this problem by making it bus
>> agnostic.
> Given how little information there is in the original patch as to exactly
> what problem this is addressing, I could be getting the wrong end of the
> stick here.
>
> Is this about trying to have a way to obtain the bus local addresses
> associated with CPU-view resources?
>
> If so, how about looking towards PCI, which has had this problem for the
> last 15+ years, where PCI bus addresses are not necessarily the same as
> CPU physical addresses?
>
> There, we don't end up with multiple addresses specified in resources.
> We instead have a way to translate between resources and bus-local
> addresses, which IMHO is far nicer and less error-prone than having to
> specify the same information twice, once with an offset and once without.
>
Not really. This is about giving the address of a sub device on a pmic
to a platform driver for that sub device. There is no CPU view. The
addresses are offsets in a register space for a PMIC or other MFD that
lives on i2c/spi or some similar sort of bus. So perhaps 0x20
corresponds to the start of the register space for an RTC and 0x38
corresponds to the start of the register space for a regulator.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project