2014-11-06 20:21:12

by DATACOM - Erico Nunes

[permalink] [raw]
Subject: Creating a new platform_bus inside a spi_driver


Hello,

In our board we have a FPGA whose programmable logic communicates with the main
processor via SPI. Inside the programmable logic we also have a range of
registers (writable via SPI commands) which implement functionality of several
distinct "devices".

Our approach to implement a driver framework for these devices was to have a
platform_bus embedded in a spi_device. This device tree source draft
illustrates the proposal:

{
spi-master@2000 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "cpu-spi-master";
reg = <0x2000 0x2000>;

fpga@1 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "fpga-spi";
reg = <1>;
spi-max-frequency = <25000000>;

fpga-device1@200 {
compatible = "fpga-device1-driver";
reg = <0x200 0x300>;
};

fpga-device2@500 {
compatible = "fpga-device2-driver";
reg = <0x500 0x100>;
};

fpga-device3@600 {
compatible = "fpga-device3-driver";
reg = <0x600 0x200>;
};
};
};
}

The idea is that "fpga-spi" is a spi_driver which instantiates all of the
"fpga-deviceN" as platform_devices, through the use of
of_platform_populate(dev->of_node, NULL, NULL, dev).

The visible problem we're facing with this approach is that, as the internal
platform_devices have a "reg" property, of_platform_populate() eventually
triggers an address translation which is apparently trying to translate the
addresses of the internal platform_bus to addresses of the processor memory
map.
This translation is however not part of our intention, as we intend to have an
internal bus with its own memory map.
This fails when __of_translate_address() reaches the spi-master boundary
because (as it seems to make sense) it isn't possible to translate them past
that.
A KERN_ERR rated message like
"prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
is thrown by __of_translate_address() and later it is not possible to obtain
the "reg" address with platform_get_resource().

On this scenario, we have a few questions and, depending on the outcome of
these, possibly a patch.

1. Is it possible to have an internal platform_bus with a different memory map
as we intended? Or are platform_busses and platform_devices supposed to always
be mapped on the processor memory map?

2. If platform_bus and platform_device were actually designed to always be
mappable to the processor memory map, what would be a different approach to
this problem? One alternative considered was to define a new "fpga_bus" and
"fpga_device" but that seemed as an overkill approach to the problem.

3. By searching for the ocurrences of of_platform_populate(), some uses which
look similar to our approach were found, in the following drivers:
drivers/mfd/smsc-ece1099.c
drivers/mfd/palmas.c
These have an i2c_driver which calls of_platform_populate() just like we
attempted. Won't they run into a similar problem, if any of the children
platform_devices provide a "reg" property?

4. By applying the patch attached at the end of this mail, things start to work
as we intended. Basically instead of returning an error if we reach an
untranslatable boundary, we return the resulting address so far. This is
assuming that, for this case, we want an internal memory map for this device.
Is this patch/assumption breaking any design decisions regarding the platform
subsystem? It is of course not a widely tested patch, but could we be facing a
bug which needs a fix like the one presented?

Thanks in advance!

Erico



[PATCH] of/address: return partial translation when crossing
untranslatable boundary

Signed-off-by: erico.nunes <[email protected]>
---
drivers/of/address.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index afdb782..ab56d31 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -572,8 +572,9 @@ static u64 __of_translate_address(struct device_node *dev,
pbus = of_match_bus(parent);
pbus->count_cells(dev, &pna, &pns);
if (!OF_CHECK_COUNTS(pna, pns)) {
- printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
+ pr_debug("OF: prom_parse: Bad cell count for %s\n",
of_node_full_name(dev));
+ result = of_read_number(addr, na);
break;
}

--
1.9.1


2014-11-07 10:04:45

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Creating a new platform_bus inside a spi_driver

On Thursday 06 November 2014 18:02:52 DATACOM - ?rico Nunes wrote:
>
> The idea is that "fpga-spi" is a spi_driver which instantiates all of the
> "fpga-deviceN" as platform_devices, through the use of
> of_platform_populate(dev->of_node, NULL, NULL, dev).
>
> The visible problem we're facing with this approach is that, as the internal
> platform_devices have a "reg" property, of_platform_populate() eventually
> triggers an address translation which is apparently trying to translate the
> addresses of the internal platform_bus to addresses of the processor memory
> map.
> This translation is however not part of our intention, as we intend to have an
> internal bus with its own memory map.
> This fails when __of_translate_address() reaches the spi-master boundary
> because (as it seems to make sense) it isn't possible to translate them past
> that.
> A KERN_ERR rated message like
> "prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
> is thrown by __of_translate_address() and later it is not possible to obtain
> the "reg" address with platform_get_resource().
>
> On this scenario, we have a few questions and, depending on the outcome of
> these, possibly a patch.
>
> 1. Is it possible to have an internal platform_bus with a different memory map
> as we intended? Or are platform_busses and platform_devices supposed to always
> be mapped on the processor memory map?

It's inconsistent. We have some code that assumes that platform devices
are always memory mapped, and some other code that breaks this assumption.

> 2. If platform_bus and platform_device were actually designed to always be
> mappable to the processor memory map, what would be a different approach to
> this problem? One alternative considered was to define a new "fpga_bus" and
> "fpga_device" but that seemed as an overkill approach to the problem.

I think the existing mfd framework should do what you need, when you call
mfd_add_devices() and pass a table of cells with the compatible strings
for your devices, it should create the platform devices you want. If not,
that can probably be fixed in the mfd core code.

Arnd

2014-11-07 16:37:31

by DATACOM - Erico Nunes

[permalink] [raw]
Subject: Re: Creating a new platform_bus inside a spi_driver

Hello Arnd and all,

On 11/07/2014 08:04 AM, Arnd Bergmann wrote:
> On Thursday 06 November 2014 18:02:52 DATACOM - ?rico Nunes wrote:
>> The idea is that "fpga-spi" is a spi_driver which instantiates all of the
>> "fpga-deviceN" as platform_devices, through the use of
>> of_platform_populate(dev->of_node, NULL, NULL, dev).
>>
>> The visible problem we're facing with this approach is that, as the internal
>> platform_devices have a "reg" property, of_platform_populate() eventually
>> triggers an address translation which is apparently trying to translate the
>> addresses of the internal platform_bus to addresses of the processor memory
>> map.
>> This translation is however not part of our intention, as we intend to have an
>> internal bus with its own memory map.
>> This fails when __of_translate_address() reaches the spi-master boundary
>> because (as it seems to make sense) it isn't possible to translate them past
>> that.
>> A KERN_ERR rated message like
>> "prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
>> is thrown by __of_translate_address() and later it is not possible to obtain
>> the "reg" address with platform_get_resource().
>>
>> On this scenario, we have a few questions and, depending on the outcome of
>> these, possibly a patch.
>>
>> 1. Is it possible to have an internal platform_bus with a different memory map
>> as we intended? Or are platform_busses and platform_devices supposed to always
>> be mapped on the processor memory map?
> It's inconsistent. We have some code that assumes that platform devices
> are always memory mapped, and some other code that breaks this assumption.

By this I take that the platform subsystem could be made generic so it can be
used in both ways (mapped to processor memory map or mapped to a private memory
map). There seems to be no strict requirement enforcing it to be processor
memory map.

Is this correct?

>> 2. If platform_bus and platform_device were actually designed to always be
>> mappable to the processor memory map, what would be a different approach to
>> this problem? One alternative considered was to define a new "fpga_bus" and
>> "fpga_device" but that seemed as an overkill approach to the problem.
> I think the existing mfd framework should do what you need, when you call
> mfd_add_devices() and pass a table of cells with the compatible strings
> for your devices, it should create the platform devices you want. If not,
> that can probably be fixed in the mfd core code.
>
>

Thanks for the tip, we were not aware of the purpose of this mfd framework and
we will take a look at this framework now.
However I'm thinking now that eventually it would fall in the same case of
trying to translate the address of any "reg" dts property to the processor
memory map, and fail with the same error for the SPI case.

Considering this and taking the answer to the first question, do you think a
patch fixing the "error" report by the translation function would be
acceptable?
We can prepare/test that under our platform and submit it.

Thanks!

2014-11-07 17:04:47

by Arnd Bergmann

[permalink] [raw]
Subject: Re: Creating a new platform_bus inside a spi_driver

On Friday 07 November 2014 14:37:26 DATACOM - ?rico Nunes wrote:
> Hello Arnd and all,
>
> On 11/07/2014 08:04 AM, Arnd Bergmann wrote:
> > On Thursday 06 November 2014 18:02:52 DATACOM - ?rico Nunes wrote:
> >> The idea is that "fpga-spi" is a spi_driver which instantiates all of the
> >> "fpga-deviceN" as platform_devices, through the use of
> >> of_platform_populate(dev->of_node, NULL, NULL, dev).
> >>
> >> The visible problem we're facing with this approach is that, as the internal
> >> platform_devices have a "reg" property, of_platform_populate() eventually
> >> triggers an address translation which is apparently trying to translate the
> >> addresses of the internal platform_bus to addresses of the processor memory
> >> map.
> >> This translation is however not part of our intention, as we intend to have an
> >> internal bus with its own memory map.
> >> This fails when __of_translate_address() reaches the spi-master boundary
> >> because (as it seems to make sense) it isn't possible to translate them past
> >> that.
> >> A KERN_ERR rated message like
> >> "prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
> >> is thrown by __of_translate_address() and later it is not possible to obtain
> >> the "reg" address with platform_get_resource().
> >>
> >> On this scenario, we have a few questions and, depending on the outcome of
> >> these, possibly a patch.
> >>
> >> 1. Is it possible to have an internal platform_bus with a different memory map
> >> as we intended? Or are platform_busses and platform_devices supposed to always
> >> be mapped on the processor memory map?
> > It's inconsistent. We have some code that assumes that platform devices
> > are always memory mapped, and some other code that breaks this assumption.
>
> By this I take that the platform subsystem could be made generic so it can be
> used in both ways (mapped to processor memory map or mapped to a private memory
> map). There seems to be no strict requirement enforcing it to be processor
> memory map.
>
> Is this correct?

It could be, but I'm sure if that is a good idea or not. It might complicate
things elsewhere, so it would at least need careful testing and consensus
among a broader group of developers.

> >> 2. If platform_bus and platform_device were actually designed to always be
> >> mappable to the processor memory map, what would be a different approach to
> >> this problem? One alternative considered was to define a new "fpga_bus" and
> >> "fpga_device" but that seemed as an overkill approach to the problem.
> > I think the existing mfd framework should do what you need, when you call
> > mfd_add_devices() and pass a table of cells with the compatible strings
> > for your devices, it should create the platform devices you want. If not,
> > that can probably be fixed in the mfd core code.
> >
> >
>
> Thanks for the tip, we were not aware of the purpose of this mfd framework and
> we will take a look at this framework now.
> However I'm thinking now that eventually it would fall in the same case of
> trying to translate the address of any "reg" dts property to the processor
> memory map, and fail with the same error for the SPI case.
>
> Considering this and taking the answer to the first question, do you think a
> patch fixing the "error" report by the translation function would be
> acceptable?
> We can prepare/test that under our platform and submit it.

Please try to use the mfd approach first. There are a lot of mfd drivers
on the SPI bus, so I'd assume this works fine.

Arnd

2014-11-11 11:08:07

by Grant Likely

[permalink] [raw]
Subject: Re: Creating a new platform_bus inside a spi_driver

On Fri, 07 Nov 2014 18:04:35 +0100
, Arnd Bergmann <[email protected]>
wrote:
> On Friday 07 November 2014 14:37:26 DATACOM - Érico Nunes wrote:
> > Hello Arnd and all,
> >
> > On 11/07/2014 08:04 AM, Arnd Bergmann wrote:
> > > On Thursday 06 November 2014 18:02:52 DATACOM - Érico Nunes wrote:
> > >> The idea is that "fpga-spi" is a spi_driver which instantiates all of the
> > >> "fpga-deviceN" as platform_devices, through the use of
> > >> of_platform_populate(dev->of_node, NULL, NULL, dev).
> > >>
> > >> The visible problem we're facing with this approach is that, as the internal
> > >> platform_devices have a "reg" property, of_platform_populate() eventually
> > >> triggers an address translation which is apparently trying to translate the
> > >> addresses of the internal platform_bus to addresses of the processor memory
> > >> map.
> > >> This translation is however not part of our intention, as we intend to have an
> > >> internal bus with its own memory map.
> > >> This fails when __of_translate_address() reaches the spi-master boundary
> > >> because (as it seems to make sense) it isn't possible to translate them past
> > >> that.
> > >> A KERN_ERR rated message like
> > >> "prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
> > >> is thrown by __of_translate_address() and later it is not possible to obtain
> > >> the "reg" address with platform_get_resource().
> > >>
> > >> On this scenario, we have a few questions and, depending on the outcome of
> > >> these, possibly a patch.
> > >>
> > >> 1. Is it possible to have an internal platform_bus with a different memory map
> > >> as we intended? Or are platform_busses and platform_devices supposed to always
> > >> be mapped on the processor memory map?
> > > It's inconsistent. We have some code that assumes that platform devices
> > > are always memory mapped, and some other code that breaks this assumption.
> >
> > By this I take that the platform subsystem could be made generic so it can be
> > used in both ways (mapped to processor memory map or mapped to a private memory
> > map). There seems to be no strict requirement enforcing it to be processor
> > memory map.
> >
> > Is this correct?
>
> It could be, but I'm sure if that is a good idea or not. It might complicate
> things elsewhere, so it would at least need careful testing and consensus
> among a broader group of developers.

I don't think it is a good idea. I would prefer to make the behaviour of
of_platform_populate() generic so it could work for multiple bus
types rather than reusing/abusing platform_device in this way.

If the devices on the FPGA were memory mapped, it would be a different
situation, but being behind an SPI bus where the access to those devices
must always be mediated by the SPI controller, it would be better to
have a separate bus type and a separate pool of drivers for those
devices.

g.

2014-11-11 11:20:37

by Grant Likely

[permalink] [raw]
Subject: Re: Creating a new platform_bus inside a spi_driver

On Thu, 06 Nov 2014 18:02:52 -0200
, DATACOM - Érico Nunes
<[email protected]>
wrote:
>
> Hello,
>
> In our board we have a FPGA whose programmable logic communicates with the main
> processor via SPI. Inside the programmable logic we also have a range of
> registers (writable via SPI commands) which implement functionality of several
> distinct "devices".
>
> Our approach to implement a driver framework for these devices was to have a
> platform_bus embedded in a spi_device. This device tree source draft
> illustrates the proposal:
>
> {
> spi-master@2000 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "cpu-spi-master";
> reg = <0x2000 0x2000>;
>
> fpga@1 {
> #address-cells = <1>;
> #size-cells = <1>;
> compatible = "fpga-spi";
> reg = <1>;
> spi-max-frequency = <25000000>;
>
> fpga-device1@200 {
> compatible = "fpga-device1-driver";
> reg = <0x200 0x300>;
> };
>
> fpga-device2@500 {
> compatible = "fpga-device2-driver";
> reg = <0x500 0x100>;
> };
>
> fpga-device3@600 {
> compatible = "fpga-device3-driver";
> reg = <0x600 0x200>;
> };
> };
> };
> }
>
> The idea is that "fpga-spi" is a spi_driver which instantiates all of the
> "fpga-deviceN" as platform_devices, through the use of
> of_platform_populate(dev->of_node, NULL, NULL, dev).
>
> The visible problem we're facing with this approach is that, as the internal
> platform_devices have a "reg" property, of_platform_populate() eventually
> triggers an address translation which is apparently trying to translate the
> addresses of the internal platform_bus to addresses of the processor memory
> map.
> This translation is however not part of our intention, as we intend to have an
> internal bus with its own memory map.
> This fails when __of_translate_address() reaches the spi-master boundary
> because (as it seems to make sense) it isn't possible to translate them past
> that.
> A KERN_ERR rated message like
> "prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
> is thrown by __of_translate_address() and later it is not possible to obtain
> the "reg" address with platform_get_resource().
>
> On this scenario, we have a few questions and, depending on the outcome of
> these, possibly a patch.
>
> 1. Is it possible to have an internal platform_bus with a different memory map
> as we intended? Or are platform_busses and platform_devices supposed to always
> be mapped on the processor memory map?
>
> 2. If platform_bus and platform_device were actually designed to always be
> mappable to the processor memory map, what would be a different approach to
> this problem? One alternative considered was to define a new "fpga_bus" and
> "fpga_device" but that seemed as an overkill approach to the problem.
>
> 3. By searching for the ocurrences of of_platform_populate(), some uses which
> look similar to our approach were found, in the following drivers:
> drivers/mfd/smsc-ece1099.c
> drivers/mfd/palmas.c
> These have an i2c_driver which calls of_platform_populate() just like we
> attempted. Won't they run into a similar problem, if any of the children
> platform_devices provide a "reg" property?
>
> 4. By applying the patch attached at the end of this mail, things start to work
> as we intended. Basically instead of returning an error if we reach an
> untranslatable boundary, we return the resulting address so far. This is
> assuming that, for this case, we want an internal memory map for this device.
> Is this patch/assumption breaking any design decisions regarding the platform
> subsystem? It is of course not a widely tested patch, but could we be facing a
> bug which needs a fix like the one presented?
>
> Thanks in advance!
>
> Erico
>
>
>
> [PATCH] of/address: return partial translation when crossing
> untranslatable boundary
>
> Signed-off-by: erico.nunes <[email protected]>
> ---
> drivers/of/address.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index afdb782..ab56d31 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -572,8 +572,9 @@ static u64 __of_translate_address(struct device_node *dev,
> pbus = of_match_bus(parent);
> pbus->count_cells(dev, &pna, &pns);
> if (!OF_CHECK_COUNTS(pna, pns)) {
> - printk(KERN_ERR "prom_parse: Bad cell count for %s\n",
> + pr_debug("OF: prom_parse: Bad cell count for %s\n",
> of_node_full_name(dev));
> + result = of_read_number(addr, na);

This will cause breakage elsewhere. The __of_translate_address()
function is intended to fail when translation is impossible. You would
need a different variant of the function, or different mode of calling
it which allows partial translation. Actually, what would be better is
to pass in an end-point for translation. You want the caller to be
explicit about where it expects the translation to stop, and still fail
if translation doesn't occur at that point. That gives some assurance
that the driver won't end up with bogus translations.

g.

2014-11-13 08:52:09

by Stanimir Varbanov

[permalink] [raw]
Subject: Re: Creating a new platform_bus inside a spi_driver

Hi Grant, Arnd and Erico

On 11/11/2014 01:07 PM, Grant Likely wrote:
> On Fri, 07 Nov 2014 18:04:35 +0100
> , Arnd Bergmann <[email protected]>
> wrote:
>> On Friday 07 November 2014 14:37:26 DATACOM - Érico Nunes wrote:
>>> Hello Arnd and all,
>>>
>>> On 11/07/2014 08:04 AM, Arnd Bergmann wrote:
>>>> On Thursday 06 November 2014 18:02:52 DATACOM - Érico Nunes wrote:
>>>>> The idea is that "fpga-spi" is a spi_driver which instantiates all of the
>>>>> "fpga-deviceN" as platform_devices, through the use of
>>>>> of_platform_populate(dev->of_node, NULL, NULL, dev).
>>>>>
>>>>> The visible problem we're facing with this approach is that, as the internal
>>>>> platform_devices have a "reg" property, of_platform_populate() eventually
>>>>> triggers an address translation which is apparently trying to translate the
>>>>> addresses of the internal platform_bus to addresses of the processor memory
>>>>> map.
>>>>> This translation is however not part of our intention, as we intend to have an
>>>>> internal bus with its own memory map.
>>>>> This fails when __of_translate_address() reaches the spi-master boundary
>>>>> because (as it seems to make sense) it isn't possible to translate them past
>>>>> that.
>>>>> A KERN_ERR rated message like
>>>>> "prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
>>>>> is thrown by __of_translate_address() and later it is not possible to obtain
>>>>> the "reg" address with platform_get_resource().
>>>>>
>>>>> On this scenario, we have a few questions and, depending on the outcome of
>>>>> these, possibly a patch.
>>>>>
>>>>> 1. Is it possible to have an internal platform_bus with a different memory map
>>>>> as we intended? Or are platform_busses and platform_devices supposed to always
>>>>> be mapped on the processor memory map?
>>>> It's inconsistent. We have some code that assumes that platform devices
>>>> are always memory mapped, and some other code that breaks this assumption.
>>>
>>> By this I take that the platform subsystem could be made generic so it can be
>>> used in both ways (mapped to processor memory map or mapped to a private memory
>>> map). There seems to be no strict requirement enforcing it to be processor
>>> memory map.
>>>
>>> Is this correct?
>>
>> It could be, but I'm sure if that is a good idea or not. It might complicate
>> things elsewhere, so it would at least need careful testing and consensus
>> among a broader group of developers.
>
> I don't think it is a good idea. I would prefer to make the behaviour of
> of_platform_populate() generic so it could work for multiple bus
> types rather than reusing/abusing platform_device in this way.
>
> If the devices on the FPGA were memory mapped, it would be a different
> situation, but being behind an SPI bus where the access to those devices
> must always be mediated by the SPI controller, it would be better to
> have a separate bus type and a separate pool of drivers for those
> devices.
>

This is exactly the same problem that we have on Qualcomm SPMI PMIC mfd
driver. We had a discussion at [1] where we tried to solve it by
IORESOURCE_REG. Unfortunately there is no conclusion yet.

I'm glad to see that someone else have the same issue, maybe it is time
to restart the discussion. The last proposal from Grant was to implement
dev_get_localbus_address() API in drivers/base.

regards
Stan

[1] https://lkml.org/lkml/2014/7/29/252

2014-11-15 15:32:11

by DATACOM - Erico Nunes

[permalink] [raw]
Subject: Re: Creating a new platform_bus inside a spi_driver



On 11/13/2014 06:52 AM, Stanimir Varbanov wrote:
> Hi Grant, Arnd and Erico
>
> On 11/11/2014 01:07 PM, Grant Likely wrote:
>> On Fri, 07 Nov 2014 18:04:35 +0100
>> , Arnd Bergmann <[email protected]>
>> wrote:
>>> On Friday 07 November 2014 14:37:26 DATACOM - Érico Nunes wrote:
>>>> Hello Arnd and all,
>>>>
>>>> On 11/07/2014 08:04 AM, Arnd Bergmann wrote:
>>>>> On Thursday 06 November 2014 18:02:52 DATACOM - Érico Nunes wrote:
>>>>>> The idea is that "fpga-spi" is a spi_driver which instantiates all of the
>>>>>> "fpga-deviceN" as platform_devices, through the use of
>>>>>> of_platform_populate(dev->of_node, NULL, NULL, dev).
>>>>>>
>>>>>> The visible problem we're facing with this approach is that, as the internal
>>>>>> platform_devices have a "reg" property, of_platform_populate() eventually
>>>>>> triggers an address translation which is apparently trying to translate the
>>>>>> addresses of the internal platform_bus to addresses of the processor memory
>>>>>> map.
>>>>>> This translation is however not part of our intention, as we intend to have an
>>>>>> internal bus with its own memory map.
>>>>>> This fails when __of_translate_address() reaches the spi-master boundary
>>>>>> because (as it seems to make sense) it isn't possible to translate them past
>>>>>> that.
>>>>>> A KERN_ERR rated message like
>>>>>> "prom_parse: Bad cell count for /soc@f0000000/spi@2000/fpga@1"
>>>>>> is thrown by __of_translate_address() and later it is not possible to obtain
>>>>>> the "reg" address with platform_get_resource().
>>>>>>
>>>>>> On this scenario, we have a few questions and, depending on the outcome of
>>>>>> these, possibly a patch.
>>>>>>
>>>>>> 1. Is it possible to have an internal platform_bus with a different memory map
>>>>>> as we intended? Or are platform_busses and platform_devices supposed to always
>>>>>> be mapped on the processor memory map?
>>>>> It's inconsistent. We have some code that assumes that platform devices
>>>>> are always memory mapped, and some other code that breaks this assumption.
>>>>
>>>> By this I take that the platform subsystem could be made generic so it can be
>>>> used in both ways (mapped to processor memory map or mapped to a private memory
>>>> map). There seems to be no strict requirement enforcing it to be processor
>>>> memory map.
>>>>
>>>> Is this correct?
>>>
>>> It could be, but I'm sure if that is a good idea or not. It might complicate
>>> things elsewhere, so it would at least need careful testing and consensus
>>> among a broader group of developers.
>>
>> I don't think it is a good idea. I would prefer to make the behaviour of
>> of_platform_populate() generic so it could work for multiple bus
>> types rather than reusing/abusing platform_device in this way.
>>
>> If the devices on the FPGA were memory mapped, it would be a different
>> situation, but being behind an SPI bus where the access to those devices
>> must always be mediated by the SPI controller, it would be better to
>> have a separate bus type and a separate pool of drivers for those
>> devices.
>>

Grant,
could you please elaborate a little on "making the behaviour of
of_platform_populate() generic so it could work for multiple bus types
without reusing/abusing platform_device"?

Initially we thought of having a separate bus type, but that felt such a
duplication of a platform_bus that we decided to try to reuse.

On the other thread line about the patch posted in my opening comment,
it was suggested that we could change __of_translate_address(), for
example to allow a translation end-point to be passed it. This seems
like a good idea to me, however with this comment here it is still not
clear to me whether this change would be likely accepted or not.

>
> This is exactly the same problem that we have on Qualcomm SPMI PMIC mfd
> driver. We had a discussion at [1] where we tried to solve it by
> IORESOURCE_REG. Unfortunately there is no conclusion yet.
>
> I'm glad to see that someone else have the same issue, maybe it is time
> to restart the discussion. The last proposal from Grant was to implement
> dev_get_localbus_address() API in drivers/base.

I took the time to read through your thread and indeed a lot of this
discussion is common. It is now clear that we should not be using
IORESOURCE_MEM as it was assumed in the original patch, which was one of
my original questions.
We are still not using the mfd framework because of the need to maintain
the table of "compatible" drivers in code, and this requires double
maintenance of code instead of adding a new device to the dts.
Nevertheless, I tested your "RFC: add function for localbus address"
patch, and your solution worked right away for my case too (without mfd)
by just changing my drivers to get IORESOURCE_REG instead.