diff mbox

[U-Boot] dm: core: Add Kconfig for simple bus driver

Message ID 1438557348-4984-1-git-send-email-marex@denx.de
State Accepted
Delegated to: Simon Glass
Headers show

Commit Message

Marek Vasut Aug. 2, 2015, 11:15 p.m. UTC
Add Kconfig entries for the simple-bus driver, both for U-Boot
and for SPL. The simple-bus is enabled by default in U-Boot and
disabled by default in SPL to preserve the original behavior.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Simon Glass <sjg@chromium.org>
---
 drivers/core/Kconfig  | 15 +++++++++++++++
 drivers/core/Makefile |  5 ++---
 2 files changed, 17 insertions(+), 3 deletions(-)

Comments

Simon Glass Aug. 2, 2015, 11:52 p.m. UTC | #1
On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
> Add Kconfig entries for the simple-bus driver, both for U-Boot
> and for SPL. The simple-bus is enabled by default in U-Boot and
> disabled by default in SPL to preserve the original behavior.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> ---
>  drivers/core/Kconfig  | 15 +++++++++++++++
>  drivers/core/Makefile |  5 ++---
>  2 files changed, 17 insertions(+), 3 deletions(-)

Acked-by: Simon Glass <sjg@chromium.org>
Marek Vasut Aug. 2, 2015, 11:55 p.m. UTC | #2
On Monday, August 03, 2015 at 01:52:03 AM, Simon Glass wrote:
> On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
> > Add Kconfig entries for the simple-bus driver, both for U-Boot
> > and for SPL. The simple-bus is enabled by default in U-Boot and
> > disabled by default in SPL to preserve the original behavior.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Simon Glass <sjg@chromium.org>
> > ---
> > 
> >  drivers/core/Kconfig  | 15 +++++++++++++++
> >  drivers/core/Makefile |  5 ++---
> >  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> Acked-by: Simon Glass <sjg@chromium.org>

Whew, excellent. That's 1 patch in, 246+ to go ... :-)

Best regards,
Marek Vasut
Simon Glass Aug. 2, 2015, 11:58 p.m. UTC | #3
Hi Marek,

On 2 August 2015 at 17:55, Marek Vasut <marex@denx.de> wrote:
> On Monday, August 03, 2015 at 01:52:03 AM, Simon Glass wrote:
>> On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
>> > Add Kconfig entries for the simple-bus driver, both for U-Boot
>> > and for SPL. The simple-bus is enabled by default in U-Boot and
>> > disabled by default in SPL to preserve the original behavior.
>> >
>> > Signed-off-by: Marek Vasut <marex@denx.de>
>> > Cc: Simon Glass <sjg@chromium.org>
>> > ---
>> >
>> >  drivers/core/Kconfig  | 15 +++++++++++++++
>> >  drivers/core/Makefile |  5 ++---
>> >  2 files changed, 17 insertions(+), 3 deletions(-)
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>
> Whew, excellent. That's 1 patch in, 246+ to go ... :-)

Chin up, old boy. You'll get there :-)

Regards,
Simon
Simon Glass Aug. 10, 2015, 1:39 p.m. UTC | #4
Hi Marek,

On 2 August 2015 at 17:52, Simon Glass <sjg@chromium.org> wrote:
> On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
>> Add Kconfig entries for the simple-bus driver, both for U-Boot
>> and for SPL. The simple-bus is enabled by default in U-Boot and
>> disabled by default in SPL to preserve the original behavior.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>  drivers/core/Kconfig  | 15 +++++++++++++++
>>  drivers/core/Makefile |  5 ++---
>>  2 files changed, 17 insertions(+), 3 deletions(-)
>
> Acked-by: Simon Glass <sjg@chromium.org>

I can't apply this right now as it breaks most tegra boards. We use a
hack at present where we #undef things in config_uncmd_spl.h and that
isn't visible to the Makefiles.

Assuming that Tom applies Masahiro's Kconfig improvements for SPL we
can do it then.

Regards,
Simon
Marek Vasut Aug. 10, 2015, 1:57 p.m. UTC | #5
On Monday, August 10, 2015 at 03:39:33 PM, Simon Glass wrote:
> Hi Marek,

Hi Simon,

> On 2 August 2015 at 17:52, Simon Glass <sjg@chromium.org> wrote:
> > On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
> >> Add Kconfig entries for the simple-bus driver, both for U-Boot
> >> and for SPL. The simple-bus is enabled by default in U-Boot and
> >> disabled by default in SPL to preserve the original behavior.
> >> 
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Simon Glass <sjg@chromium.org>
> >> ---
> >> 
> >>  drivers/core/Kconfig  | 15 +++++++++++++++
> >>  drivers/core/Makefile |  5 ++---
> >>  2 files changed, 17 insertions(+), 3 deletions(-)
> > 
> > Acked-by: Simon Glass <sjg@chromium.org>
> 
> I can't apply this right now as it breaks most tegra boards. We use a
> hack at present where we #undef things in config_uncmd_spl.h and that
> isn't visible to the Makefiles.
> 
> Assuming that Tom applies Masahiro's Kconfig improvements for SPL we
> can do it then.

Agh, OK, thanks for the heads up.

I hope this makes it into 2015.10, otherwise the SPL support for SoCFPGA
will be completely useless.

Best regards,
Marek Vasut
Simon Glass Aug. 10, 2015, 2 p.m. UTC | #6
Hi Marek,

On 10 August 2015 at 07:57, Marek Vasut <marex@denx.de> wrote:
> On Monday, August 10, 2015 at 03:39:33 PM, Simon Glass wrote:
>> Hi Marek,
>
> Hi Simon,
>
>> On 2 August 2015 at 17:52, Simon Glass <sjg@chromium.org> wrote:
>> > On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
>> >> Add Kconfig entries for the simple-bus driver, both for U-Boot
>> >> and for SPL. The simple-bus is enabled by default in U-Boot and
>> >> disabled by default in SPL to preserve the original behavior.
>> >>
>> >> Signed-off-by: Marek Vasut <marex@denx.de>
>> >> Cc: Simon Glass <sjg@chromium.org>
>> >> ---
>> >>
>> >>  drivers/core/Kconfig  | 15 +++++++++++++++
>> >>  drivers/core/Makefile |  5 ++---
>> >>  2 files changed, 17 insertions(+), 3 deletions(-)
>> >
>> > Acked-by: Simon Glass <sjg@chromium.org>
>>
>> I can't apply this right now as it breaks most tegra boards. We use a
>> hack at present where we #undef things in config_uncmd_spl.h and that
>> isn't visible to the Makefiles.
>>
>> Assuming that Tom applies Masahiro's Kconfig improvements for SPL we
>> can do it then.
>
> Agh, OK, thanks for the heads up.
>
> I hope this makes it into 2015.10, otherwise the SPL support for SoCFPGA
> will be completely useless.

One way or another we'll figure this out sooner rather than later.

Regards,
Simon
Marek Vasut Aug. 10, 2015, 2:03 p.m. UTC | #7
On Monday, August 10, 2015 at 04:00:47 PM, Simon Glass wrote:
> Hi Marek,

Hi Simon,

> On 10 August 2015 at 07:57, Marek Vasut <marex@denx.de> wrote:
> > On Monday, August 10, 2015 at 03:39:33 PM, Simon Glass wrote:
> >> Hi Marek,
> > 
> > Hi Simon,
> > 
> >> On 2 August 2015 at 17:52, Simon Glass <sjg@chromium.org> wrote:
> >> > On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
> >> >> Add Kconfig entries for the simple-bus driver, both for U-Boot
> >> >> and for SPL. The simple-bus is enabled by default in U-Boot and
> >> >> disabled by default in SPL to preserve the original behavior.
> >> >> 
> >> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> >> Cc: Simon Glass <sjg@chromium.org>
> >> >> ---
> >> >> 
> >> >>  drivers/core/Kconfig  | 15 +++++++++++++++
> >> >>  drivers/core/Makefile |  5 ++---
> >> >>  2 files changed, 17 insertions(+), 3 deletions(-)
> >> > 
> >> > Acked-by: Simon Glass <sjg@chromium.org>
> >> 
> >> I can't apply this right now as it breaks most tegra boards. We use a
> >> hack at present where we #undef things in config_uncmd_spl.h and that
> >> isn't visible to the Makefiles.
> >> 
> >> Assuming that Tom applies Masahiro's Kconfig improvements for SPL we
> >> can do it then.
> > 
> > Agh, OK, thanks for the heads up.
> > 
> > I hope this makes it into 2015.10, otherwise the SPL support for SoCFPGA
> > will be completely useless.
> 
> One way or another we'll figure this out sooner rather than later.

I know :)

Best regards,
Marek Vasut
Simon Glass Aug. 22, 2015, 1:40 p.m. UTC | #8
On 10 August 2015 at 08:03, Marek Vasut <marex@denx.de> wrote:
> On Monday, August 10, 2015 at 04:00:47 PM, Simon Glass wrote:
>> Hi Marek,
>
> Hi Simon,
>
>> On 10 August 2015 at 07:57, Marek Vasut <marex@denx.de> wrote:
>> > On Monday, August 10, 2015 at 03:39:33 PM, Simon Glass wrote:
>> >> Hi Marek,
>> >
>> > Hi Simon,
>> >
>> >> On 2 August 2015 at 17:52, Simon Glass <sjg@chromium.org> wrote:
>> >> > On 2 August 2015 at 17:15, Marek Vasut <marex@denx.de> wrote:
>> >> >> Add Kconfig entries for the simple-bus driver, both for U-Boot
>> >> >> and for SPL. The simple-bus is enabled by default in U-Boot and
>> >> >> disabled by default in SPL to preserve the original behavior.
>> >> >>
>> >> >> Signed-off-by: Marek Vasut <marex@denx.de>
>> >> >> Cc: Simon Glass <sjg@chromium.org>
>> >> >> ---
>> >> >>
>> >> >>  drivers/core/Kconfig  | 15 +++++++++++++++
>> >> >>  drivers/core/Makefile |  5 ++---
>> >> >>  2 files changed, 17 insertions(+), 3 deletions(-)
>> >> >
>> >> > Acked-by: Simon Glass <sjg@chromium.org>
>> >>
>> >> I can't apply this right now as it breaks most tegra boards. We use a
>> >> hack at present where we #undef things in config_uncmd_spl.h and that
>> >> isn't visible to the Makefiles.
>> >>
>> >> Assuming that Tom applies Masahiro's Kconfig improvements for SPL we
>> >> can do it then.
>> >
>> > Agh, OK, thanks for the heads up.
>> >
>> > I hope this makes it into 2015.10, otherwise the SPL support for SoCFPGA
>> > will be completely useless.
>>
>> One way or another we'll figure this out sooner rather than later.
>
> I know :)

Masahiro's series landed so I have applied this to u-boot-dm.

Please check that it works as you expect.

Regards,
Simon
Marek Vasut Aug. 22, 2015, 3:50 p.m. UTC | #9
On Saturday, August 22, 2015 at 03:40:26 PM, Simon Glass wrote:

Hi!

[...]

> >> One way or another we'll figure this out sooner rather than later.
> > 
> > I know :)
> 
> Masahiro's series landed so I have applied this to u-boot-dm.
> 
> Please check that it works as you expect.

I think we're having a minor duplicity here now. Check the 
drivers/core/Makefile, there's this bit already:

 obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o

And since I have both CONFIG_OF_CONTROL and CONFIG_SPL_OF_CONTROL, I
get the simple bus driver as well.

But I don't think that the current state is entirely correct. I propose
the remove the above obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o from
the Makefile to fix the duplicity and achieve the behavior you wanted to
have. What do you think please ? I can send a quick patch.

Best regards,
Marek Vasut
Simon Glass Aug. 22, 2015, 4:01 p.m. UTC | #10
Hi Marek,

On 22 August 2015 at 09:50, Marek Vasut <marex@denx.de> wrote:
> On Saturday, August 22, 2015 at 03:40:26 PM, Simon Glass wrote:
>
> Hi!
>
> [...]
>
>> >> One way or another we'll figure this out sooner rather than later.
>> >
>> > I know :)
>>
>> Masahiro's series landed so I have applied this to u-boot-dm.
>>
>> Please check that it works as you expect.
>
> I think we're having a minor duplicity here now. Check the
> drivers/core/Makefile, there's this bit already:
>
>  obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
>
> And since I have both CONFIG_OF_CONTROL and CONFIG_SPL_OF_CONTROL, I
> get the simple bus driver as well.
>
> But I don't think that the current state is entirely correct. I propose
> the remove the above obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o from
> the Makefile to fix the duplicity and achieve the behavior you wanted to
> have. What do you think please ? I can send a quick patch.

Actually I think that is just a merge error that I made. Can you
please check it now?

Regards,
Simon
Marek Vasut Aug. 22, 2015, 4:48 p.m. UTC | #11
On Saturday, August 22, 2015 at 06:01:44 PM, Simon Glass wrote:
> Hi Marek,

Hi!

> On 22 August 2015 at 09:50, Marek Vasut <marex@denx.de> wrote:
> > On Saturday, August 22, 2015 at 03:40:26 PM, Simon Glass wrote:
> > 
> > Hi!
> > 
> > [...]
> > 
> >> >> One way or another we'll figure this out sooner rather than later.
> >> > 
> >> > I know :)
> >> 
> >> Masahiro's series landed so I have applied this to u-boot-dm.
> >> 
> >> Please check that it works as you expect.
> > 
> > I think we're having a minor duplicity here now. Check the
> > 
> > drivers/core/Makefile, there's this bit already:
> >  obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
> > 
> > And since I have both CONFIG_OF_CONTROL and CONFIG_SPL_OF_CONTROL, I
> > get the simple bus driver as well.
> > 
> > But I don't think that the current state is entirely correct. I propose
> > the remove the above obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o from
> > the Makefile to fix the duplicity and achieve the behavior you wanted to
> > have. What do you think please ? I can send a quick patch.
> 
> Actually I think that is just a merge error that I made. Can you
> please check it now?

I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now depend on
SPL_OF_CONTROL, not on OF_CONTROL, right ? :)

Best regards,
Marek Vasut
Simon Glass Aug. 22, 2015, 5:09 p.m. UTC | #12
Hi Marek,

On 22 August 2015 at 10:48, Marek Vasut <marex@denx.de> wrote:
> On Saturday, August 22, 2015 at 06:01:44 PM, Simon Glass wrote:
>> Hi Marek,
>
> Hi!
>
>> On 22 August 2015 at 09:50, Marek Vasut <marex@denx.de> wrote:
>> > On Saturday, August 22, 2015 at 03:40:26 PM, Simon Glass wrote:
>> >
>> > Hi!
>> >
>> > [...]
>> >
>> >> >> One way or another we'll figure this out sooner rather than later.
>> >> >
>> >> > I know :)
>> >>
>> >> Masahiro's series landed so I have applied this to u-boot-dm.
>> >>
>> >> Please check that it works as you expect.
>> >
>> > I think we're having a minor duplicity here now. Check the
>> >
>> > drivers/core/Makefile, there's this bit already:
>> >  obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
>> >
>> > And since I have both CONFIG_OF_CONTROL and CONFIG_SPL_OF_CONTROL, I
>> > get the simple bus driver as well.
>> >
>> > But I don't think that the current state is entirely correct. I propose
>> > the remove the above obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o from
>> > the Makefile to fix the duplicity and achieve the behavior you wanted to
>> > have. What do you think please ? I can send a quick patch.
>>
>> Actually I think that is just a merge error that I made. Can you
>> please check it now?
>
> I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now depend on
> SPL_OF_CONTROL, not on OF_CONTROL, right ? :)

Ah yes that sounds right. Would you like to send a new patch against
mainline? I think it is best to consider this a rewrite rather than a
merge!

Regards,
Simon
Marek Vasut Aug. 23, 2015, 1:12 a.m. UTC | #13
On Saturday, August 22, 2015 at 07:09:21 PM, Simon Glass wrote:
> Hi Marek,

Hi Simon,

> On 22 August 2015 at 10:48, Marek Vasut <marex@denx.de> wrote:
> > On Saturday, August 22, 2015 at 06:01:44 PM, Simon Glass wrote:
> >> Hi Marek,
> > 
> > Hi!
> > 
> >> On 22 August 2015 at 09:50, Marek Vasut <marex@denx.de> wrote:
> >> > On Saturday, August 22, 2015 at 03:40:26 PM, Simon Glass wrote:
> >> > 
> >> > Hi!
> >> > 
> >> > [...]
> >> > 
> >> >> >> One way or another we'll figure this out sooner rather than later.
> >> >> > 
> >> >> > I know :)
> >> >> 
> >> >> Masahiro's series landed so I have applied this to u-boot-dm.
> >> >> 
> >> >> Please check that it works as you expect.
> >> > 
> >> > I think we're having a minor duplicity here now. Check the
> >> > 
> >> > drivers/core/Makefile, there's this bit already:
> >> >  obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
> >> > 
> >> > And since I have both CONFIG_OF_CONTROL and CONFIG_SPL_OF_CONTROL, I
> >> > get the simple bus driver as well.
> >> > 
> >> > But I don't think that the current state is entirely correct. I
> >> > propose the remove the above obj-$(CONFIG_$(SPL_)OF_CONTROL) +=
> >> > simple-bus.o from the Makefile to fix the duplicity and achieve the
> >> > behavior you wanted to have. What do you think please ? I can send a
> >> > quick patch.
> >> 
> >> Actually I think that is just a merge error that I made. Can you
> >> please check it now?
> > 
> > I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now
> > depend on SPL_OF_CONTROL, not on OF_CONTROL, right ? :)
> 
> Ah yes that sounds right. Would you like to send a new patch against
> mainline? I think it is best to consider this a rewrite rather than a
> merge!

Heh :) I think that's the last missing bit, so feel free to fix it and
we're good. But if you insist on a new patch, please do let me know :)

Best regards,
Marek Vasut
Simon Glass Aug. 23, 2015, 9:21 p.m. UTC | #14
Hi Marek,

On 22 August 2015 at 19:12, Marek Vasut <marex@denx.de> wrote:
> On Saturday, August 22, 2015 at 07:09:21 PM, Simon Glass wrote:
>> Hi Marek,
>
> Hi Simon,
>
>> On 22 August 2015 at 10:48, Marek Vasut <marex@denx.de> wrote:
>> > On Saturday, August 22, 2015 at 06:01:44 PM, Simon Glass wrote:
>> >> Hi Marek,
>> >
>> > Hi!
>> >
>> >> On 22 August 2015 at 09:50, Marek Vasut <marex@denx.de> wrote:
>> >> > On Saturday, August 22, 2015 at 03:40:26 PM, Simon Glass wrote:
>> >> >
>> >> > Hi!
>> >> >
>> >> > [...]
>> >> >
>> >> >> >> One way or another we'll figure this out sooner rather than later.
>> >> >> >
>> >> >> > I know :)
>> >> >>
>> >> >> Masahiro's series landed so I have applied this to u-boot-dm.
>> >> >>
>> >> >> Please check that it works as you expect.
>> >> >
>> >> > I think we're having a minor duplicity here now. Check the
>> >> >
>> >> > drivers/core/Makefile, there's this bit already:
>> >> >  obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
>> >> >
>> >> > And since I have both CONFIG_OF_CONTROL and CONFIG_SPL_OF_CONTROL, I
>> >> > get the simple bus driver as well.
>> >> >
>> >> > But I don't think that the current state is entirely correct. I
>> >> > propose the remove the above obj-$(CONFIG_$(SPL_)OF_CONTROL) +=
>> >> > simple-bus.o from the Makefile to fix the duplicity and achieve the
>> >> > behavior you wanted to have. What do you think please ? I can send a
>> >> > quick patch.
>> >>
>> >> Actually I think that is just a merge error that I made. Can you
>> >> please check it now?
>> >
>> > I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now
>> > depend on SPL_OF_CONTROL, not on OF_CONTROL, right ? :)
>>
>> Ah yes that sounds right. Would you like to send a new patch against
>> mainline? I think it is best to consider this a rewrite rather than a
>> merge!
>
> Heh :) I think that's the last missing bit, so feel free to fix it and
> we're good. But if you insist on a new patch, please do let me know :)

OK, done. Let me know how it looks.

Regards,
Simon
Marek Vasut Aug. 24, 2015, 2:54 a.m. UTC | #15
On Sunday, August 23, 2015 at 11:21:27 PM, Simon Glass wrote:
> Hi Marek,

Hi Simon,

[...]

> >> > I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now
> >> > depend on SPL_OF_CONTROL, not on OF_CONTROL, right ? :)
> >> 
> >> Ah yes that sounds right. Would you like to send a new patch against
> >> mainline? I think it is best to consider this a rewrite rather than a
> >> merge!
> > 
> > Heh :) I think that's the last missing bit, so feel free to fix it and
> > we're good. But if you insist on a new patch, please do let me know :)
> 
> OK, done. Let me know how it looks.

Looks great, thanks :)

Best regards,
Marek Vasut
Masahiro Yamada Aug. 24, 2015, 10:02 a.m. UTC | #16
2015-08-24 11:54 GMT+09:00 Marek Vasut <marex@denx.de>:
> On Sunday, August 23, 2015 at 11:21:27 PM, Simon Glass wrote:
>> Hi Marek,
>
> Hi Simon,
>
> [...]
>
>> >> > I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now
>> >> > depend on SPL_OF_CONTROL, not on OF_CONTROL, right ? :)
>> >>
>> >> Ah yes that sounds right. Would you like to send a new patch against
>> >> mainline? I think it is best to consider this a rewrite rather than a
>> >> merge!
>> >
>> > Heh :) I think that's the last missing bit, so feel free to fix it and
>> > we're good. But if you insist on a new patch, please do let me know :)
>>
>> OK, done. Let me know how it looks.
>
> Looks great, thanks :)
>


Nope.


If CONFIG_OF_CONTROL is enabled and CONFIG_SIMPLE_BUS is disabled,
I get build error.


  LD      u-boot
drivers/built-in.o: In function `dev_get_addr':
/home/yamada/ref/u-boot-dm/drivers/core/device.c:571: undefined
reference to `simple_bus_translate'
collect2: error: ld returned 1 exit status
make: *** [u-boot] Error 1


Do you really want to switch OF_CONTROL and SIMPLE_BUS separately?

simple-bus.c looks like a part of DM core rather than a driver because
the address transformation (simple_bus_translate) is mandatory to
handle reg properties in DTS correctly.

The reason for disabling SIMPLE_BUS but not OF_CONTROL, if any,
is to save memory footprint of drivers/core/simple-bus.c
Do you think it is significant?


I think this is enough:

obj-y += device.o lists.o root.o uclass.o util.o
obj-$(CONFIG_DEVRES) += devres.o
obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE) += device-remove.o
obj-$(CONFIG_DM) += dump.o
obj-$(CONFIG_REGMAP) += regmap.o
obj-$(CONFIG_SYSCON) += syscon-uclass.o

(drop "ifndef CONFIG_SPL_BUILD" and "endif")
Marek Vasut Aug. 24, 2015, 10:14 a.m. UTC | #17
On Monday, August 24, 2015 at 12:02:08 PM, Masahiro Yamada wrote:
> 2015-08-24 11:54 GMT+09:00 Marek Vasut <marex@denx.de>:
> > On Sunday, August 23, 2015 at 11:21:27 PM, Simon Glass wrote:
> >> Hi Marek,
> > 
> > Hi Simon,
> > 
> > [...]
> > 
> >> >> > I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now
> >> >> > depend on SPL_OF_CONTROL, not on OF_CONTROL, right ? :)
> >> >> 
> >> >> Ah yes that sounds right. Would you like to send a new patch against
> >> >> mainline? I think it is best to consider this a rewrite rather than a
> >> >> merge!
> >> > 
> >> > Heh :) I think that's the last missing bit, so feel free to fix it and
> >> > we're good. But if you insist on a new patch, please do let me know :)
> >> 
> >> OK, done. Let me know how it looks.
> > 
> > Looks great, thanks :)
> 
> Nope.
> 
> 
> If CONFIG_OF_CONTROL is enabled and CONFIG_SIMPLE_BUS is disabled,
> I get build error.

Hrm.

>   LD      u-boot
> drivers/built-in.o: In function `dev_get_addr':
> /home/yamada/ref/u-boot-dm/drivers/core/device.c:571: undefined
> reference to `simple_bus_translate'
> collect2: error: ld returned 1 exit status
> make: *** [u-boot] Error 1
> 
> 
> Do you really want to switch OF_CONTROL and SIMPLE_BUS separately?

I don't need it anymore in fact. I just needed to have simple-bus support
enabled in SPL, which I have even without this patch now. This is how it
came to be:

https://www.mail-archive.com/u-boot@lists.denx.de/msg179851.html

> simple-bus.c looks like a part of DM core rather than a driver because
> the address transformation (simple_bus_translate) is mandatory to
> handle reg properties in DTS correctly.
> 
> The reason for disabling SIMPLE_BUS but not OF_CONTROL, if any,
> is to save memory footprint of drivers/core/simple-bus.c
> Do you think it is significant?
> 
> 
> I think this is enough:
> 
> obj-y += device.o lists.o root.o uclass.o util.o
> obj-$(CONFIG_DEVRES) += devres.o
> obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
> obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE) += device-remove.o
> obj-$(CONFIG_DM) += dump.o
> obj-$(CONFIG_REGMAP) += regmap.o
> obj-$(CONFIG_SYSCON) += syscon-uclass.o
> 
> (drop "ifndef CONFIG_SPL_BUILD" and "endif")

Feel free to apply the original patch ;-)

https://www.mail-archive.com/u-boot@lists.denx.de/msg178872.html

Best regards,
Marek Vasut
Simon Glass Aug. 24, 2015, 4:58 p.m. UTC | #18
Hi Masahiro,

On 24 August 2015 at 04:02, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2015-08-24 11:54 GMT+09:00 Marek Vasut <marex@denx.de>:
>> On Sunday, August 23, 2015 at 11:21:27 PM, Simon Glass wrote:
>>> Hi Marek,
>>
>> Hi Simon,
>>
>> [...]
>>
>>> >> > I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now
>>> >> > depend on SPL_OF_CONTROL, not on OF_CONTROL, right ? :)
>>> >>
>>> >> Ah yes that sounds right. Would you like to send a new patch against
>>> >> mainline? I think it is best to consider this a rewrite rather than a
>>> >> merge!
>>> >
>>> > Heh :) I think that's the last missing bit, so feel free to fix it and
>>> > we're good. But if you insist on a new patch, please do let me know :)
>>>
>>> OK, done. Let me know how it looks.
>>
>> Looks great, thanks :)
>>
>
>
> Nope.
>
>
> If CONFIG_OF_CONTROL is enabled and CONFIG_SIMPLE_BUS is disabled,
> I get build error.
>
>
>   LD      u-boot
> drivers/built-in.o: In function `dev_get_addr':
> /home/yamada/ref/u-boot-dm/drivers/core/device.c:571: undefined
> reference to `simple_bus_translate'
> collect2: error: ld returned 1 exit status
> make: *** [u-boot] Error 1
>
>
> Do you really want to switch OF_CONTROL and SIMPLE_BUS separately?
>
> simple-bus.c looks like a part of DM core rather than a driver because
> the address transformation (simple_bus_translate) is mandatory to
> handle reg properties in DTS correctly.
>
> The reason for disabling SIMPLE_BUS but not OF_CONTROL, if any,
> is to save memory footprint of drivers/core/simple-bus.c
> Do you think it is significant?

It's not large but it was enough for me to to split it out as an
option recently. I'd like to avoid needless bloat particularly where
it affects SPL.

We can fix that build error - it's just that we have the wrong condition:

#ifndef CONFIG_SPL_BUILD
if (device_get_uclass_id(dev->parent) == UCLASS_SIMPLE_BUS)
addr = simple_bus_translate(dev->parent, addr);
#endif

But I think this patch needs to be redone and resent since I am
effectively rewriting it in the name of merge conflicts. I'll take a
look.

>
>
> I think this is enough:
>
> obj-y += device.o lists.o root.o uclass.o util.o
> obj-$(CONFIG_DEVRES) += devres.o
> obj-$(CONFIG_$(SPL_)OF_CONTROL) += simple-bus.o
> obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE) += device-remove.o
> obj-$(CONFIG_DM) += dump.o
> obj-$(CONFIG_REGMAP) += regmap.o
> obj-$(CONFIG_SYSCON) += syscon-uclass.o
>
> (drop "ifndef CONFIG_SPL_BUILD" and "endif")

Regards,
Simon
Simon Glass Aug. 30, 2015, 10:42 p.m. UTC | #19
Hi,

On 24 August 2015 at 10:58, Simon Glass <sjg@chromium.org> wrote:
> Hi Masahiro,
>
> On 24 August 2015 at 04:02, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> 2015-08-24 11:54 GMT+09:00 Marek Vasut <marex@denx.de>:
>>> On Sunday, August 23, 2015 at 11:21:27 PM, Simon Glass wrote:
>>>> Hi Marek,
>>>
>>> Hi Simon,
>>>
>>> [...]
>>>
>>>> >> > I think the in drivers/core/Kconfig , the SPL_SIMPLE_BUS should now
>>>> >> > depend on SPL_OF_CONTROL, not on OF_CONTROL, right ? :)
>>>> >>
>>>> >> Ah yes that sounds right. Would you like to send a new patch against
>>>> >> mainline? I think it is best to consider this a rewrite rather than a
>>>> >> merge!
>>>> >
>>>> > Heh :) I think that's the last missing bit, so feel free to fix it and
>>>> > we're good. But if you insist on a new patch, please do let me know :)
>>>>
>>>> OK, done. Let me know how it looks.
>>>
>>> Looks great, thanks :)
>>>
>>
>>
>> Nope.
>>
>>
>> If CONFIG_OF_CONTROL is enabled and CONFIG_SIMPLE_BUS is disabled,
>> I get build error.
>>
>>
>>   LD      u-boot
>> drivers/built-in.o: In function `dev_get_addr':
>> /home/yamada/ref/u-boot-dm/drivers/core/device.c:571: undefined
>> reference to `simple_bus_translate'
>> collect2: error: ld returned 1 exit status
>> make: *** [u-boot] Error 1
>>
>>
>> Do you really want to switch OF_CONTROL and SIMPLE_BUS separately?
>>
>> simple-bus.c looks like a part of DM core rather than a driver because
>> the address transformation (simple_bus_translate) is mandatory to
>> handle reg properties in DTS correctly.
>>
>> The reason for disabling SIMPLE_BUS but not OF_CONTROL, if any,
>> is to save memory footprint of drivers/core/simple-bus.c
>> Do you think it is significant?
>
> It's not large but it was enough for me to to split it out as an
> option recently. I'd like to avoid needless bloat particularly where
> it affects SPL.
>
> We can fix that build error - it's just that we have the wrong condition:
>
> #ifndef CONFIG_SPL_BUILD
> if (device_get_uclass_id(dev->parent) == UCLASS_SIMPLE_BUS)
> addr = simple_bus_translate(dev->parent, addr);
> #endif
>
> But I think this patch needs to be redone and resent since I am
> effectively rewriting it in the name of merge conflicts. I'll take a
> look.
>

OK I've had another go at this, so if there are any other problems /
changes, please submit a patch!

Applied (again) to u-boot-dm, thanks!

- Simon
diff mbox

Patch

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index e40372d..3403d1f 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -59,3 +59,18 @@  config DM_SEQ_ALIAS
 	  Most boards will have a '/aliases' node containing the path to
 	  numbered devices (e.g. serial0 = &serial0). This feature can be
 	  disabled if it is not required, to save code space in SPL.
+
+config SIMPLE_BUS
+	bool "Support simple-bus driver"
+	depends on DM && OF_CONTROL
+	default y
+	help
+	  Supports the 'simple-bus' driver, which is used on some systems.
+
+config SPL_SIMPLE_BUS
+	bool "Support simple-bus driver in SPL"
+	depends on SPL_DM && OF_CONTROL
+	default n
+	help
+	  Supports the 'simple-bus' driver, which is used on some systems
+	  in SPL.
diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 5c2ead8..74588b1 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -5,9 +5,8 @@ 
 #
 
 obj-y	+= device.o lists.o root.o uclass.o util.o
-ifndef CONFIG_SPL_BUILD
-obj-$(CONFIG_OF_CONTROL) += simple-bus.o
-endif
+obj-$(CONFIG_SIMPLE_BUS)	+= simple-bus.o
+obj-$(CONFIG_SPL_SIMPLE_BUS)	+= simple-bus.o
 obj-$(CONFIG_DM_DEVICE_REMOVE)	+= device-remove.o
 obj-$(CONFIG_DM)	+= dump.o
 obj-$(CONFIG_OF_CONTROL)	+= regmap.o