| Message ID | 1627909100-83338-4-git-send-email-Sanju.Mehta@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for AMD USB4 and bug fixes | expand |
On Mon, Aug 02, 2021 at 07:58:19AM -0500, Sanjay R Mehta wrote: > From: Sanjay R Mehta <sanju.mehta@amd.com> > > Adapter0 (Port0) is the control adapter on the AMD USB4 host router. > As per USB4 spec in "Section 1.8", Control Adapters do not > have an Adapter Configuration Space". > > The read requests on Adapter0 time's out and driver initialization fails. > > Hence Disabling the Adapter in case of read-request timeout and continuing > the driver init. > > Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> > Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > --- > drivers/thunderbolt/switch.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > index 83b1ef3..effbfe4 100644 > --- a/drivers/thunderbolt/switch.c > +++ b/drivers/thunderbolt/switch.c > @@ -2747,8 +2747,9 @@ int tb_switch_add(struct tb_switch *sw) > } > ret = tb_init_port(&sw->ports[i]); > if (ret) { > + sw->ports[i].disabled = true; > dev_err(&sw->dev, "failed to initialize port %d\n", i); > - return ret; > + continue; Instead of this, would it work if we start the loop at 1? In case of the control adapter (0) tb_port_init() does not do anything useful anyway and it actually would simplify that function too if we get rid of the special casing. > } > } > > -- > 2.7.4
On 8/2/2021 8:56 PM, Mika Westerberg wrote: > [CAUTION: External Email] > > On Mon, Aug 02, 2021 at 07:58:19AM -0500, Sanjay R Mehta wrote: >> From: Sanjay R Mehta <sanju.mehta@amd.com> >> >> Adapter0 (Port0) is the control adapter on the AMD USB4 host router. >> As per USB4 spec in "Section 1.8", Control Adapters do not >> have an Adapter Configuration Space". >> >> The read requests on Adapter0 time's out and driver initialization fails. >> >> Hence Disabling the Adapter in case of read-request timeout and continuing >> the driver init. >> >> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> >> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> >> --- >> drivers/thunderbolt/switch.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c >> index 83b1ef3..effbfe4 100644 >> --- a/drivers/thunderbolt/switch.c >> +++ b/drivers/thunderbolt/switch.c >> @@ -2747,8 +2747,9 @@ int tb_switch_add(struct tb_switch *sw) >> } >> ret = tb_init_port(&sw->ports[i]); >> if (ret) { >> + sw->ports[i].disabled = true; >> dev_err(&sw->dev, "failed to initialize port %d\n", i); >> - return ret; >> + continue; > > Instead of this, would it work if we start the loop at 1? In case of the > control adapter (0) tb_port_init() does not do anything useful anyway > and it actually would simplify that function too if we get rid of the > special casing. > Hi Mika, If we start loop from 1, it will work for host router but this will skip port (0) on device router which may be valid port. Yes, as you said, for host router adapter (0) we can skip tb_port_init(). Will send the changes accordingly. Thanks, Sanjay >> } >> } >> >> -- >> 2.7.4
Hi, On Tue, Aug 03, 2021 at 12:23:44AM +0530, Sanjay R Mehta wrote: > > > On 8/2/2021 8:56 PM, Mika Westerberg wrote: > > [CAUTION: External Email] > > > > On Mon, Aug 02, 2021 at 07:58:19AM -0500, Sanjay R Mehta wrote: > >> From: Sanjay R Mehta <sanju.mehta@amd.com> > >> > >> Adapter0 (Port0) is the control adapter on the AMD USB4 host router. > >> As per USB4 spec in "Section 1.8", Control Adapters do not > >> have an Adapter Configuration Space". > >> > >> The read requests on Adapter0 time's out and driver initialization fails. > >> > >> Hence Disabling the Adapter in case of read-request timeout and continuing > >> the driver init. > >> > >> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> > >> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> > >> --- > >> drivers/thunderbolt/switch.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c > >> index 83b1ef3..effbfe4 100644 > >> --- a/drivers/thunderbolt/switch.c > >> +++ b/drivers/thunderbolt/switch.c > >> @@ -2747,8 +2747,9 @@ int tb_switch_add(struct tb_switch *sw) > >> } > >> ret = tb_init_port(&sw->ports[i]); > >> if (ret) { > >> + sw->ports[i].disabled = true; > >> dev_err(&sw->dev, "failed to initialize port %d\n", i); > >> - return ret; > >> + continue; > > > > Instead of this, would it work if we start the loop at 1? In case of the > > control adapter (0) tb_port_init() does not do anything useful anyway > > and it actually would simplify that function too if we get rid of the > > special casing. > > > Hi Mika, > > If we start loop from 1, it will work for host router > but this will skip port (0) on device router which may be valid port. For device router adapter 0 is also contror adapter so I think we can just skip it here unconditionally.
On 8/3/2021 2:35 PM, Mika Westerberg wrote: > [CAUTION: External Email] > > Hi, > > On Tue, Aug 03, 2021 at 12:23:44AM +0530, Sanjay R Mehta wrote: >> >> >> On 8/2/2021 8:56 PM, Mika Westerberg wrote: >>> [CAUTION: External Email] >>> >>> On Mon, Aug 02, 2021 at 07:58:19AM -0500, Sanjay R Mehta wrote: >>>> From: Sanjay R Mehta <sanju.mehta@amd.com> >>>> >>>> Adapter0 (Port0) is the control adapter on the AMD USB4 host router. >>>> As per USB4 spec in "Section 1.8", Control Adapters do not >>>> have an Adapter Configuration Space". >>>> >>>> The read requests on Adapter0 time's out and driver initialization fails. >>>> >>>> Hence Disabling the Adapter in case of read-request timeout and continuing >>>> the driver init. >>>> >>>> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com> >>>> Signed-off-by: Sanjay R Mehta <sanju.mehta@amd.com> >>>> --- >>>> drivers/thunderbolt/switch.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c >>>> index 83b1ef3..effbfe4 100644 >>>> --- a/drivers/thunderbolt/switch.c >>>> +++ b/drivers/thunderbolt/switch.c >>>> @@ -2747,8 +2747,9 @@ int tb_switch_add(struct tb_switch *sw) >>>> } >>>> ret = tb_init_port(&sw->ports[i]); >>>> if (ret) { >>>> + sw->ports[i].disabled = true; >>>> dev_err(&sw->dev, "failed to initialize port %d\n", i); >>>> - return ret; >>>> + continue; >>> >>> Instead of this, would it work if we start the loop at 1? In case of the >>> control adapter (0) tb_port_init() does not do anything useful anyway >>> and it actually would simplify that function too if we get rid of the >>> special casing. >>> >> Hi Mika, >> >> If we start loop from 1, it will work for host router >> but this will skip port (0) on device router which may be valid port. > > For device router adapter 0 is also contror adapter so I think we can > just skip it here unconditionally. Sure Mika. Will send the updated changes. Thanks, Sanjay >
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c index 83b1ef3..effbfe4 100644 --- a/drivers/thunderbolt/switch.c +++ b/drivers/thunderbolt/switch.c @@ -2747,8 +2747,9 @@ int tb_switch_add(struct tb_switch *sw) } ret = tb_init_port(&sw->ports[i]); if (ret) { + sw->ports[i].disabled = true; dev_err(&sw->dev, "failed to initialize port %d\n", i); - return ret; + continue; } }