Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Marek Vasut]

On 11/20/2018 09:01 PM, Sven Schwermer wrote:
>> Well, the easiest test might be to run this through buildman, since it
>> reports if there was any size increase for any board.
> 
> Isn’t that run during the Travis CI builds? If so, I’ve done it:
> https://travis-ci.org/svenschwermer/u-boot/builds/456862718

Is it ? I wasn't aware of that, I don't think so.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Sven Schwermer]

> Well, the easiest test might be to run this through buildman, since it
> reports if there was any size increase for any board.

Isn’t that run during the Travis CI builds? If so, I’ve done it:
https://travis-ci.org/svenschwermer/u-boot/builds/456862718

Sven
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Marek Vasut]

On 11/20/2018 06:42 PM, Sven Schwermer wrote:
> Hi,
> 
>> On 20 Nov 2018, at 16:47, Marek Vasut  wrote:
>>
>> On 11/20/2018 04:21 PM, Sven Schwermer wrote:
>>> Hi again,
>>>
 On 20 Nov 2018, at 15:14, Marek Vasut >>> > wrote:

 On 11/20/2018 03:10 PM, Sven Schwermer wrote:
>
>>> Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think
>>> that’s the case.
>>
>> Are you _sure_ ? Sorry if I'm being hard, but this affects a lot of
>> users, so I have to be cautious.
>
> I am _not_ sure. That’s why I used the verb “think” ;-) I’m sorry,
> but I don’t think I can provide the guarantees you need. How can we
> investigate this systematically?

 I think you can just run make $board_defconfig for all boards (I don't
 think you need toolchain to do that) and verify that the option didn't
 get enabled in the resulting .config .
>>>
>>> I can confirm that after applying this series, no board has
>>> CONFIG_SPL_DM_USB=y that does not have CONFIG_DM_USB=y. Why is it
>>> necessary to check this? Do we not trust the Kconfig dependencies? Or
>>> did I misunderstand you and that’s what you wanted me to check?
>>
>> But DM_USB is DM and USB in U-Boot proper and it doesn't imply that SPL
>> should have DM or USB support. I want to be sure that some boards don't
>> suddenly grow DM or USB support in SPL if it wasn't there before.
> 
> If anything, this patch series takes away DM_USB from SPL where it was 
> previously active. Since SPL_DM_USB depends on DM_USB, it won’t be enabled 
> where it wasn’t enabled already. Before this series, DM_USB was enabled also 
> in the SPL when enabled in proper.
> 
> Sorry, I guess I’m missing something fundamental here. I fail to see the 
> possibility of code being included in the SPL that wasn’t included before.

Well, the easiest test might be to run this through buildman, since it
reports if there was any size increase for any board.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Sven Schwermer]

Hi,

> On 20 Nov 2018, at 16:47, Marek Vasut  wrote:
> 
> On 11/20/2018 04:21 PM, Sven Schwermer wrote:
>> Hi again,
>> 
>>> On 20 Nov 2018, at 15:14, Marek Vasut >> > wrote:
>>> 
>>> On 11/20/2018 03:10 PM, Sven Schwermer wrote:
 
>> Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think
>> that’s the case.
> 
> Are you _sure_ ? Sorry if I'm being hard, but this affects a lot of
> users, so I have to be cautious.
 
 I am _not_ sure. That’s why I used the verb “think” ;-) I’m sorry,
 but I don’t think I can provide the guarantees you need. How can we
 investigate this systematically?
>>> 
>>> I think you can just run make $board_defconfig for all boards (I don't
>>> think you need toolchain to do that) and verify that the option didn't
>>> get enabled in the resulting .config .
>> 
>> I can confirm that after applying this series, no board has
>> CONFIG_SPL_DM_USB=y that does not have CONFIG_DM_USB=y. Why is it
>> necessary to check this? Do we not trust the Kconfig dependencies? Or
>> did I misunderstand you and that’s what you wanted me to check?
> 
> But DM_USB is DM and USB in U-Boot proper and it doesn't imply that SPL
> should have DM or USB support. I want to be sure that some boards don't
> suddenly grow DM or USB support in SPL if it wasn't there before.

If anything, this patch series takes away DM_USB from SPL where it was 
previously active. Since SPL_DM_USB depends on DM_USB, it won’t be enabled 
where it wasn’t enabled already. Before this series, DM_USB was enabled also in 
the SPL when enabled in proper.

Sorry, I guess I’m missing something fundamental here. I fail to see the 
possibility of code being included in the SPL that wasn’t included before.

Sven
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Marek Vasut]

On 11/20/2018 04:21 PM, Sven Schwermer wrote:
> Hi again,
> 
>> On 20 Nov 2018, at 15:14, Marek Vasut > > wrote:
>>
>> On 11/20/2018 03:10 PM, Sven Schwermer wrote:
>>>
> Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think
> that’s the case.

 Are you _sure_ ? Sorry if I'm being hard, but this affects a lot of
 users, so I have to be cautious.
>>>
>>> I am _not_ sure. That’s why I used the verb “think” ;-) I’m sorry,
>>> but I don’t think I can provide the guarantees you need. How can we
>>> investigate this systematically?
>>
>> I think you can just run make $board_defconfig for all boards (I don't
>> think you need toolchain to do that) and verify that the option didn't
>> get enabled in the resulting .config .
> 
> I can confirm that after applying this series, no board has
> CONFIG_SPL_DM_USB=y that does not have CONFIG_DM_USB=y. Why is it
> necessary to check this? Do we not trust the Kconfig dependencies? Or
> did I misunderstand you and that’s what you wanted me to check?

But DM_USB is DM and USB in U-Boot proper and it doesn't imply that SPL
should have DM or USB support. I want to be sure that some boards don't
suddenly grow DM or USB support in SPL if it wasn't there before.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Sven Schwermer]

Hi again,

> On 20 Nov 2018, at 15:14, Marek Vasut  wrote:
> 
> On 11/20/2018 03:10 PM, Sven Schwermer wrote:
>> 
 Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think that’s 
 the case.
>>> 
>>> Are you _sure_ ? Sorry if I'm being hard, but this affects a lot of
>>> users, so I have to be cautious.
>> 
>> I am _not_ sure. That’s why I used the verb “think” ;-) I’m sorry, but I 
>> don’t think I can provide the guarantees you need. How can we investigate 
>> this systematically?
> 
> I think you can just run make $board_defconfig for all boards (I don't
> think you need toolchain to do that) and verify that the option didn't
> get enabled in the resulting .config .

I can confirm that after applying this series, no board has CONFIG_SPL_DM_USB=y 
that does not have CONFIG_DM_USB=y. Why is it necessary to check this? Do we 
not trust the Kconfig dependencies? Or did I misunderstand you and that’s what 
you wanted me to check?

Sven
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Marek Vasut]

On 11/20/2018 03:10 PM, Sven Schwermer wrote:
> 
>>> Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think that’s 
>>> the case.
>>
>> Are you _sure_ ? Sorry if I'm being hard, but this affects a lot of
>> users, so I have to be cautious.
> 
> I am _not_ sure. That’s why I used the verb “think” ;-) I’m sorry, but I 
> don’t think I can provide the guarantees you need. How can we investigate 
> this systematically?

I think you can just run make $board_defconfig for all boards (I don't
think you need toolchain to do that) and verify that the option didn't
get enabled in the resulting .config .

 btw I don't think SPL_DM_USB should depend on DM_USB , one is for SPL
 and the other for U-Boot.
>>>
>>> Aha! There are two reasons I did this:
>>> 1) I used CONFIG_SPL_DM_MMC as the inspiration for my changes. That one 
>>> depends on CONFIG_DM_MMC.
>>
>> I don't think that's right. SPL and U-Boot are separate things.
> 
> Agreed.
> 
>>> 2) In order to not break a lot of configs, see above.
>>>
>>> If this is unacceptable for you, I guess, I’ll have to revisit this. That’d 
>>> make this series a lot bigger I think. Tell me your preference.
>>
>> I don't mind bigger and better series, but let's just answer the
>> question first.
> 
> I’m just afraid, that this is more likely to break existing boards.

Let's see :)

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Sven Schwermer]

>> Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think that’s 
>> the case.
> 
> Are you _sure_ ? Sorry if I'm being hard, but this affects a lot of
> users, so I have to be cautious.

I am _not_ sure. That’s why I used the verb “think” ;-) I’m sorry, but I don’t 
think I can provide the guarantees you need. How can we investigate this 
systematically?

> 
>>> btw I don't think SPL_DM_USB should depend on DM_USB , one is for SPL
>>> and the other for U-Boot.
>> 
>> Aha! There are two reasons I did this:
>> 1) I used CONFIG_SPL_DM_MMC as the inspiration for my changes. That one 
>> depends on CONFIG_DM_MMC.
> 
> I don't think that's right. SPL and U-Boot are separate things.

Agreed.

>> 2) In order to not break a lot of configs, see above.
>> 
>> If this is unacceptable for you, I guess, I’ll have to revisit this. That’d 
>> make this series a lot bigger I think. Tell me your preference.
> 
> I don't mind bigger and better series, but let's just answer the
> question first.

I’m just afraid, that this is more likely to break existing boards.

Sven
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Marek Vasut]

On 11/20/2018 02:22 PM, Sven Schwermer wrote:
> This configuration doesn't use USB in the SPL, so we need to disable
> driver model for USB in the SPL.
>
> Signed-off-by: Sven Schwermer 

 Is this also needed on other boards ?
>>>
>>> As per the cover letter, I have successfully run the Travis CI build for 
>>> this patch series. Furthermore, I have manually verified that all AM33xx 
>>> builds still succeed. So I don’t think, we need to change anything on other 
>>> boards. If you want me to do more testing, let me know.
>>
>> I just want to know whether some boards now get CONFIG_SPL_DM_USB even
>> though they didn't have it before, since the Kconfig option is default=y
> 
> Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think that’s the 
> case.

Are you _sure_ ? Sorry if I'm being hard, but this affects a lot of
users, so I have to be cautious.

>> btw I don't think SPL_DM_USB should depend on DM_USB , one is for SPL
>> and the other for U-Boot.
> 
> Aha! There are two reasons I did this:
> 1) I used CONFIG_SPL_DM_MMC as the inspiration for my changes. That one 
> depends on CONFIG_DM_MMC.

I don't think that's right. SPL and U-Boot are separate things.

> 2) In order to not break a lot of configs, see above.
> 
> If this is unacceptable for you, I guess, I’ll have to revisit this. That’d 
> make this series a lot bigger I think. Tell me your preference.

I don't mind bigger and better series, but let's just answer the
question first.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Sven Schwermer]

 This configuration doesn't use USB in the SPL, so we need to disable
 driver model for USB in the SPL.
 
 Signed-off-by: Sven Schwermer 
>>> 
>>> Is this also needed on other boards ?
>> 
>> As per the cover letter, I have successfully run the Travis CI build for 
>> this patch series. Furthermore, I have manually verified that all AM33xx 
>> builds still succeed. So I don’t think, we need to change anything on other 
>> boards. If you want me to do more testing, let me know.
> 
> I just want to know whether some boards now get CONFIG_SPL_DM_USB even
> though they didn't have it before, since the Kconfig option is default=y

Well, since CONFIG_SPL_DM_USB depends on SPL_DM_USB, I don’t think that’s the 
case.

> btw I don't think SPL_DM_USB should depend on DM_USB , one is for SPL
> and the other for U-Boot.

Aha! There are two reasons I did this:
1) I used CONFIG_SPL_DM_MMC as the inspiration for my changes. That one depends 
on CONFIG_DM_MMC.
2) In order to not break a lot of configs, see above.

If this is unacceptable for you, I guess, I’ll have to revisit this. That’d 
make this series a lot bigger I think. Tell me your preference.

Sven
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Marek Vasut]

On 11/20/2018 08:28 AM, Sven Schwermer wrote:
>>> This configuration doesn't use USB in the SPL, so we need to disable
>>> driver model for USB in the SPL.
>>>
>>> Signed-off-by: Sven Schwermer 
>>
>> Is this also needed on other boards ?
> 
> As per the cover letter, I have successfully run the Travis CI build for this 
> patch series. Furthermore, I have manually verified that all AM33xx builds 
> still succeed. So I don’t think, we need to change anything on other boards. 
> If you want me to do more testing, let me know.

I just want to know whether some boards now get CONFIG_SPL_DM_USB even
though they didn't have it before, since the Kconfig option is default=y

btw I don't think SPL_DM_USB should depend on DM_USB , one is for SPL
and the other for U-Boot.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Sven Schwermer]

>> This configuration doesn't use USB in the SPL, so we need to disable
>> driver model for USB in the SPL.
>> 
>> Signed-off-by: Sven Schwermer 
> 
> Is this also needed on other boards ?

As per the cover letter, I have successfully run the Travis CI build for this 
patch series. Furthermore, I have manually verified that all AM33xx builds 
still succeed. So I don’t think, we need to change anything on other boards. If 
you want me to do more testing, let me know.

Best regards,
Sven
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Marek Vasut]

On 11/16/2018 12:04 PM, Sven Schwermer wrote:
> This configuration doesn't use USB in the SPL, so we need to disable
> driver model for USB in the SPL.
> 
> Signed-off-by: Sven Schwermer 

Is this also needed on other boards ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 2/5] usb: am335x_evm: Disable CONFIG_SPL_DM_USB

[Sven Schwermer]

This configuration doesn't use USB in the SPL, so we need to disable
driver model for USB in the SPL.

Signed-off-by: Sven Schwermer 
---
 configs/am335x_evm_defconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configs/am335x_evm_defconfig b/configs/am335x_evm_defconfig
index b6cd49a469..6cc170ad40 100644
--- a/configs/am335x_evm_defconfig
+++ b/configs/am335x_evm_defconfig
@@ -40,15 +40,16 @@ CONFIG_NAND=y
 CONFIG_SPI_FLASH=y
 CONFIG_SPI_FLASH_WINBOND=y
 CONFIG_DM_ETH=y
-CONFIG_DRIVER_TI_CPSW=y
 CONFIG_PHY_GIGE=y
 CONFIG_MII=y
+CONFIG_DRIVER_TI_CPSW=y
 CONFIG_SPI=y
 CONFIG_OMAP3_SPI=y
 CONFIG_TIMER=y
 CONFIG_OMAP_TIMER=y
 CONFIG_USB=y
 CONFIG_DM_USB=y
+# CONFIG_SPL_DM_USB is not set
 CONFIG_USB_MUSB_HOST=y
 CONFIG_USB_MUSB_GADGET=y
 CONFIG_USB_MUSB_TI=y
-- 
2.17.1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot