For multi-function endpoint device, an ep function shouldn't stop EP
controller. Nomally the controller is stopped via configfs.
Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
Signed-off-by: Shunsuke Mie <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 5b833f00e980..a5ed779b0a51 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
cancel_delayed_work(&epf_test->cmd_handler);
pci_epf_test_clean_dma_chan(epf_test);
- pci_epc_stop(epc);
for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
epf_bar = &epf->bar[bar];
--
2.17.1
On 22/06/22 09:39, Shunsuke Mie wrote:
> For multi-function endpoint device, an ep function shouldn't stop EP
> controller. Nomally the controller is stopped via configfs.
%s/Nomally/Normally/
>
> Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
> Signed-off-by: Shunsuke Mie <[email protected]>
Thank you for the fix!
Acked-by: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 5b833f00e980..a5ed779b0a51 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>
> cancel_delayed_work(&epf_test->cmd_handler);
> pci_epf_test_clean_dma_chan(epf_test);
> - pci_epc_stop(epc);
> for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> epf_bar = &epf->bar[bar];
>
Hi Kishon,
Thank you for your ack.
2022年6月22日(水) 14:10 Kishon Vijay Abraham I <[email protected]>:
>
>
>
> On 22/06/22 09:39, Shunsuke Mie wrote:
> > For multi-function endpoint device, an ep function shouldn't stop EP
> > controller. Nomally the controller is stopped via configfs.
>
> %s/Nomally/Normally/
Oops, sorry. Should I resend this patch with fixing the typo?
> > Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
> > Signed-off-by: Shunsuke Mie <[email protected]>
>
> Thank you for the fix!
>
> Acked-by: Kishon Vijay Abraham I <[email protected]>
> > ---
> > drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 5b833f00e980..a5ed779b0a51 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
> >
> > cancel_delayed_work(&epf_test->cmd_handler);
> > pci_epf_test_clean_dma_chan(epf_test);
> > - pci_epc_stop(epc);
> > for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> > epf_bar = &epf->bar[bar];
> >
Thanks.
Shunsuke Mie
On Wed, Jun 22, 2022 at 01:09:24PM +0900, Shunsuke Mie wrote:
> For multi-function endpoint device, an ep function shouldn't stop EP
> controller. Nomally the controller is stopped via configfs.
>
> Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
> Signed-off-by: Shunsuke Mie <[email protected]>
Fixed typo and applied with Kishon's ack to pci/endpoint for v5.20, thanks!
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 5b833f00e980..a5ed779b0a51 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>
> cancel_delayed_work(&epf_test->cmd_handler);
> pci_epf_test_clean_dma_chan(epf_test);
> - pci_epc_stop(epc);
> for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> epf_bar = &epf->bar[bar];
>
> --
> 2.17.1
>
On Wed, Jun 22, 2022 at 01:09:24PM +0900, Shunsuke Mie wrote:
> For multi-function endpoint device, an ep function shouldn't stop EP
> controller. Nomally the controller is stopped via configfs.
Can you please clarify this for me?
An endpoint function by itself wouldn't stop an endpoint controller.
I assume that some *operation* on an endpoint function currently stops
the endpoint controller, but that operation should not stop the
controller?
I guess the operation is an "unbind" that detaches an EPF device from
an EPC device?
> Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
> Signed-off-by: Shunsuke Mie <[email protected]>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 5b833f00e980..a5ed779b0a51 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
>
> cancel_delayed_work(&epf_test->cmd_handler);
> pci_epf_test_clean_dma_chan(epf_test);
> - pci_epc_stop(epc);
> for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> epf_bar = &epf->bar[bar];
>
> --
> 2.17.1
>
2022年7月6日(水) 7:40 Bjorn Helgaas <[email protected]>:
>
> On Wed, Jun 22, 2022 at 01:09:24PM +0900, Shunsuke Mie wrote:
> > For multi-function endpoint device, an ep function shouldn't stop EP
> > controller. Nomally the controller is stopped via configfs.
>
> Can you please clarify this for me?
>
> An endpoint function by itself wouldn't stop an endpoint controller.
> I assume that some *operation* on an endpoint function currently stops
> the endpoint controller, but that operation should not stop the
> controller?
>
> I guess the operation is an "unbind" that detaches an EPF device from
> an EPC device?
It is likely that after all of the endpoint functions are unbound, the
controller can be stopped safely, but I'm not sure if it is desired behavior
for endpoint framework.
Kishon, could you please comment?
> > Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
> > Signed-off-by: Shunsuke Mie <[email protected]>
> > ---
> > drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index 5b833f00e980..a5ed779b0a51 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
> >
> > cancel_delayed_work(&epf_test->cmd_handler);
> > pci_epf_test_clean_dma_chan(epf_test);
> > - pci_epc_stop(epc);
> > for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> > epf_bar = &epf->bar[bar];
> >
> > --
> > 2.17.1
> >
Thanks,
Shunsuke
On Wed, Jul 06, 2022 at 11:37:29AM +0900, Shunsuke Mie wrote:
> 2022年7月6日(水) 7:40 Bjorn Helgaas <[email protected]>:
> > On Wed, Jun 22, 2022 at 01:09:24PM +0900, Shunsuke Mie wrote:
> > > For multi-function endpoint device, an ep function shouldn't stop EP
> > > controller. Nomally the controller is stopped via configfs.
> >
> > Can you please clarify this for me?
> >
> > An endpoint function by itself wouldn't stop an endpoint controller.
> > I assume that some *operation* on an endpoint function currently stops
> > the endpoint controller, but that operation should not stop the
> > controller?
> >
> > I guess the operation is an "unbind" that detaches an EPF device from
> > an EPC device?
>
> It is likely that after all of the endpoint functions are unbound, the
> controller can be stopped safely, but I'm not sure if it is desired behavior
> for endpoint framework.
I'm not asking about the patch itself. I'm asking about the commit
log because "an EP function shouldn't stop EP controller" doesn't
quite make sense in English.
I suspect it should say something like "unbinding one endpoint
function of a multi-function device from the endpoint controller
should not stop the controller."
But I don't know enough about EPF/EPC binding to know whether that
makes sense either.
> Kishon, could you please comment?
>
> > > Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
> > > Signed-off-by: Shunsuke Mie <[email protected]>
> > > ---
> > > drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > index 5b833f00e980..a5ed779b0a51 100644
> > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > @@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
> > >
> > > cancel_delayed_work(&epf_test->cmd_handler);
> > > pci_epf_test_clean_dma_chan(epf_test);
> > > - pci_epc_stop(epc);
> > > for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> > > epf_bar = &epf->bar[bar];
> > >
> > > --
> > > 2.17.1
> > >
>
> Thanks,
> Shunsuke
2022年7月6日(水) 12:08 Bjorn Helgaas <[email protected]>:
>
> On Wed, Jul 06, 2022 at 11:37:29AM +0900, Shunsuke Mie wrote:
> > 2022年7月6日(水) 7:40 Bjorn Helgaas <[email protected]>:
> > > On Wed, Jun 22, 2022 at 01:09:24PM +0900, Shunsuke Mie wrote:
> > > > For multi-function endpoint device, an ep function shouldn't stop EP
> > > > controller. Nomally the controller is stopped via configfs.
> > >
> > > Can you please clarify this for me?
> > >
> > > An endpoint function by itself wouldn't stop an endpoint controller.
> > > I assume that some *operation* on an endpoint function currently stops
> > > the endpoint controller, but that operation should not stop the
> > > controller?
> > >
> > > I guess the operation is an "unbind" that detaches an EPF device from
> > > an EPC device?
> >
> > It is likely that after all of the endpoint functions are unbound, the
> > controller can be stopped safely, but I'm not sure if it is desired behavior
> > for endpoint framework.
>
> I'm not asking about the patch itself. I'm asking about the commit
> log because "an EP function shouldn't stop EP controller" doesn't
> quite make sense in English.
I'm sorry.
> I suspect it should say something like "unbinding one endpoint
> function of a multi-function device from the endpoint controller
> should not stop the controller."
Yes, it is correct and represents the commit clearly.
> But I don't know enough about EPF/EPC binding to know whether that
> makes sense either.
>
> > Kishon, could you please comment?
> >
> > > > Fixes: 349e7a85b25f ("PCI: endpoint: functions: Add an EP function to test PCI")
> > > > Signed-off-by: Shunsuke Mie <[email protected]>
> > > > ---
> > > > drivers/pci/endpoint/functions/pci-epf-test.c | 1 -
> > > > 1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > > index 5b833f00e980..a5ed779b0a51 100644
> > > > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > > > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > > > @@ -627,7 +627,6 @@ static void pci_epf_test_unbind(struct pci_epf *epf)
> > > >
> > > > cancel_delayed_work(&epf_test->cmd_handler);
> > > > pci_epf_test_clean_dma_chan(epf_test);
> > > > - pci_epc_stop(epc);
> > > > for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
> > > > epf_bar = &epf->bar[bar];
> > > >
> > > > --
> > > > 2.17.1
> > > >
> >
> > Thanks,
> > Shunsuke
On Wed, Jul 06, 2022 at 12:15:38PM +0900, Shunsuke Mie wrote:
> 2022年7月6日(水) 12:08 Bjorn Helgaas <[email protected]>:
> > On Wed, Jul 06, 2022 at 11:37:29AM +0900, Shunsuke Mie wrote:
> > > 2022年7月6日(水) 7:40 Bjorn Helgaas <[email protected]>:
> > > > On Wed, Jun 22, 2022 at 01:09:24PM +0900, Shunsuke Mie wrote:
> > > > > For multi-function endpoint device, an ep function shouldn't stop EP
> > > > > controller. Nomally the controller is stopped via configfs.
> > > >
> > > > Can you please clarify this for me?
> > > >
> > > > An endpoint function by itself wouldn't stop an endpoint controller.
> > > > I assume that some *operation* on an endpoint function currently stops
> > > > the endpoint controller, but that operation should not stop the
> > > > controller?
> > > >
> > > > I guess the operation is an "unbind" that detaches an EPF device from
> > > > an EPC device?
> > >
> > > It is likely that after all of the endpoint functions are unbound, the
> > > controller can be stopped safely, but I'm not sure if it is desired behavior
> > > for endpoint framework.
> >
> > I'm not asking about the patch itself. I'm asking about the commit
> > log because "an EP function shouldn't stop EP controller" doesn't
> > quite make sense in English.
> I'm sorry.
>
> > I suspect it should say something like "unbinding one endpoint
> > function of a multi-function device from the endpoint controller
> > should not stop the controller."
> Yes, it is correct and represents the commit clearly.
Thanks! I updated the commit log to the following:
PCI: endpoint: Don't stop controller when unbinding endpoint function
Unbinding an endpoint function from the endpoint controller shouldn't stop
the controller. This is especially a problem for multi-function endpoints
where other endpoints may still be active.
Don't stop the controller when unbinding one of its endpoints. Normally
the controller is stopped via configfs.