diff mbox

[U-Boot,2/8] dm: pci: Avoid a driver model build error with CONFIG_CMD_PCI_ENUM

Message ID 1445104205-4079-3-git-send-email-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass Oct. 17, 2015, 5:49 p.m. UTC
This is not supported with driver model, so print a message instead of
generating a build error. Rescanning PCI is not yet implemented.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/cmd_pci.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Stephen Warren Oct. 21, 2015, 8:16 p.m. UTC | #1
On 10/17/2015 11:49 AM, Simon Glass wrote:
> This is not supported with driver model, so print a message instead of
> generating a build error. Rescanning PCI is not yet implemented.

> diff --git a/common/cmd_pci.c b/common/cmd_pci.c

> @@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>   		return pci_cfg_display(bdf, addr, size, value);
>   #ifdef CONFIG_CMD_PCI_ENUM
>   	case 'e':
> +# ifdef CONFIG_DM_PCI
> +		printf("This command is not yet supported with driver model\n");
> +# else
>   		pci_init();
> +# endif

That feature is enabled on most/all Tegra boards with PCI support. 
Hopefully nobody will miss it; I guess I haven't used it so I don't 
object to this change.

However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the 
config header rather than leaving the command enabled yet 
non-functional? Or are you planning on implementing this path in the 
near future?
Simon Glass Oct. 23, 2015, 3:47 p.m. UTC | #2
Hi Stephen,

On 21 October 2015 at 14:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 10/17/2015 11:49 AM, Simon Glass wrote:
>>
>> This is not supported with driver model, so print a message instead of
>> generating a build error. Rescanning PCI is not yet implemented.
>
>
>> diff --git a/common/cmd_pci.c b/common/cmd_pci.c
>
>
>> @@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int
>> argc, char * const argv[])
>>                 return pci_cfg_display(bdf, addr, size, value);
>>   #ifdef CONFIG_CMD_PCI_ENUM
>>         case 'e':
>> +# ifdef CONFIG_DM_PCI
>> +               printf("This command is not yet supported with driver
>> model\n");
>> +# else
>>                 pci_init();
>> +# endif
>
>
> That feature is enabled on most/all Tegra boards with PCI support. Hopefully
> nobody will miss it; I guess I haven't used it so I don't object to this
> change.
>
> However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the config
> header rather than leaving the command enabled yet non-functional? Or are
> you planning on implementing this path in the near future?

I was looking for feedback on why anyone would use this option. It's
not clear to me what it is for.

We can implement it for driver model if it is needed.

Regards,
Simon
Stephen Warren Oct. 23, 2015, 5:30 p.m. UTC | #3
On 10/23/2015 09:47 AM, Simon Glass wrote:
> Hi Stephen,
>
> On 21 October 2015 at 14:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 10/17/2015 11:49 AM, Simon Glass wrote:
>>>
>>> This is not supported with driver model, so print a message instead of
>>> generating a build error. Rescanning PCI is not yet implemented.
>>
>>
>>> diff --git a/common/cmd_pci.c b/common/cmd_pci.c
>>
>>
>>> @@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int
>>> argc, char * const argv[])
>>>                  return pci_cfg_display(bdf, addr, size, value);
>>>    #ifdef CONFIG_CMD_PCI_ENUM
>>>          case 'e':
>>> +# ifdef CONFIG_DM_PCI
>>> +               printf("This command is not yet supported with driver
>>> model\n");
>>> +# else
>>>                  pci_init();
>>> +# endif
>>
>>
>> That feature is enabled on most/all Tegra boards with PCI support. Hopefully
>> nobody will miss it; I guess I haven't used it so I don't object to this
>> change.
>>
>> However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the config
>> header rather than leaving the command enabled yet non-functional? Or are
>> you planning on implementing this path in the near future?
>
> I was looking for feedback on why anyone would use this option. It's
> not clear to me what it is for.

I assume it's to support hot-pluggable PCI connectors? Or perhaps it's 
just useful during debug of PCI drivers?

> We can implement it for driver model if it is needed.

I'd be happy disabling it in the configs myself. Thierry, you enabled 
the option when you first implemented the Tegra PCIe controller driver. 
Do you have a preference?
Thierry Reding Nov. 9, 2015, 2:33 p.m. UTC | #4
On Fri, Oct 23, 2015 at 11:30:04AM -0600, Stephen Warren wrote:
> On 10/23/2015 09:47 AM, Simon Glass wrote:
> >Hi Stephen,
> >
> >On 21 October 2015 at 14:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >>On 10/17/2015 11:49 AM, Simon Glass wrote:
> >>>
> >>>This is not supported with driver model, so print a message instead of
> >>>generating a build error. Rescanning PCI is not yet implemented.
> >>
> >>
> >>>diff --git a/common/cmd_pci.c b/common/cmd_pci.c
> >>
> >>
> >>>@@ -457,7 +457,11 @@ static int do_pci(cmd_tbl_t *cmdtp, int flag, int
> >>>argc, char * const argv[])
> >>>                 return pci_cfg_display(bdf, addr, size, value);
> >>>   #ifdef CONFIG_CMD_PCI_ENUM
> >>>         case 'e':
> >>>+# ifdef CONFIG_DM_PCI
> >>>+               printf("This command is not yet supported with driver
> >>>model\n");
> >>>+# else
> >>>                 pci_init();
> >>>+# endif
> >>
> >>
> >>That feature is enabled on most/all Tegra boards with PCI support. Hopefully
> >>nobody will miss it; I guess I haven't used it so I don't object to this
> >>change.
> >>
> >>However, wouldn't it be better to remove CONFIG_CMD_PCI_ENUM from the config
> >>header rather than leaving the command enabled yet non-functional? Or are
> >>you planning on implementing this path in the near future?
> >
> >I was looking for feedback on why anyone would use this option. It's
> >not clear to me what it is for.
> 
> I assume it's to support hot-pluggable PCI connectors? Or perhaps it's just
> useful during debug of PCI drivers?
> 
> >We can implement it for driver model if it is needed.
> 
> I'd be happy disabling it in the configs myself. Thierry, you enabled the
> option when you first implemented the Tegra PCIe controller driver. Do you
> have a preference?

Removing it from the configs is fine with me. I don't think I've
actually ever used it since all the way back when I was working on the
initial driver prototype.

Thierry
diff mbox

Patch

diff --git a/common/cmd_pci.c b/common/cmd_pci.c
index dcecef8..69c5332 100644
--- a/common/cmd_pci.c
+++ b/common/cmd_pci.c
@@ -457,7 +457,11 @@  static int do_pci(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		return pci_cfg_display(bdf, addr, size, value);
 #ifdef CONFIG_CMD_PCI_ENUM
 	case 'e':
+# ifdef CONFIG_DM_PCI
+		printf("This command is not yet supported with driver model\n");
+# else
 		pci_init();
+# endif
 		return 0;
 #endif
 	case 'n':		/* next */