mbox series

[V1,0/3] Add notifier call chain to Embedded USB Debug(EUD) driver

Message ID cover.1689148711.git.quic_schowdhu@quicinc.com (mailing list archive)
Headers show
Series Add notifier call chain to Embedded USB Debug(EUD) driver | expand

Message

Souradeep Chowdhury July 12, 2023, 8:22 a.m. UTC
This patch series adds the notifier chain to the Embedded USB Debug(EUD) driver.
The notifier chain is used to check the role switch status of EUD. Since EUD can
function only in device mode, other modules trying to do role-switch on the same
port have to first check the EUD status by calling this notifier chain and based
on the status proceed or block their role-switching step. The modules can call
the notifier through the call eud_notifier_call_chain and pass their own
role switch state as the argument. This chain will also be able to handle the
scenario of multiple modules switching roles on the same port since this can
create a priority and ordering among them for conflict resolution.

Souradeep Chowdhury (3):
  usb: misc: Add the interface for notifier call for Embedded USB
    Debugger(EUD)
  usb: misc: Add notifier call chain to Embedded USB Debug(EUD) driver
  MAINTAINERS: Add the header file entry for Embedded USB debugger(EUD)

 MAINTAINERS                          |  1 +
 drivers/usb/misc/qcom_eud.c          | 52 ++++++++++++++++++++++++++--
 drivers/usb/misc/qcom_eud_notifier.h | 10 ++++++
 3 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 drivers/usb/misc/qcom_eud_notifier.h

--
2.17.1

Comments

Greg Kroah-Hartman July 12, 2023, 4:40 p.m. UTC | #1
On Wed, Jul 12, 2023 at 01:52:37PM +0530, Souradeep Chowdhury wrote:
> This patch series adds the notifier chain to the Embedded USB Debug(EUD) driver.
> The notifier chain is used to check the role switch status of EUD. Since EUD can
> function only in device mode, other modules trying to do role-switch on the same
> port have to first check the EUD status by calling this notifier chain and based
> on the status proceed or block their role-switching step. The modules can call
> the notifier through the call eud_notifier_call_chain and pass their own
> role switch state as the argument. This chain will also be able to handle the
> scenario of multiple modules switching roles on the same port since this can
> create a priority and ordering among them for conflict resolution.

You are adding a new api that no one is actually using, so why would we
accept this at all?

And how can we actually review it without any real users?

thanks,

greg k-h
Konrad Dybcio July 12, 2023, 4:43 p.m. UTC | #2
On 12.07.2023 10:22, Souradeep Chowdhury wrote:
> This patch series adds the notifier chain to the Embedded USB Debug(EUD) driver.
> The notifier chain is used to check the role switch status of EUD. Since EUD can
> function only in device mode, other modules trying to do role-switch on the same
> port have to first check the EUD status by calling this notifier chain and based
> on the status proceed or block their role-switching step. The modules can call
> the notifier through the call eud_notifier_call_chain and pass their own
> role switch state as the argument. This chain will also be able to handle the
> scenario of multiple modules switching roles on the same port since this can
> create a priority and ordering among them for conflict resolution.
> 
> Souradeep Chowdhury (3):
>   usb: misc: Add the interface for notifier call for Embedded USB
>     Debugger(EUD)
>   usb: misc: Add notifier call chain to Embedded USB Debug(EUD) driver
>   MAINTAINERS: Add the header file entry for Embedded USB debugger(EUD)
Please actually CC all maintainers, as present in the MAINTAINERS file..

Consider using b4:

https://b4.docs.kernel.org/en/latest/index.html

Konrad
> 
>  MAINTAINERS                          |  1 +
>  drivers/usb/misc/qcom_eud.c          | 52 ++++++++++++++++++++++++++--
>  drivers/usb/misc/qcom_eud_notifier.h | 10 ++++++
>  3 files changed, 61 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/usb/misc/qcom_eud_notifier.h
> 
> --
> 2.17.1
>
Souradeep Chowdhury July 13, 2023, 8:25 a.m. UTC | #3
On 7/12/2023 10:10 PM, Greg KH wrote:
> On Wed, Jul 12, 2023 at 01:52:37PM +0530, Souradeep Chowdhury wrote:
>> This patch series adds the notifier chain to the Embedded USB Debug(EUD) driver.
>> The notifier chain is used to check the role switch status of EUD. Since EUD can
>> function only in device mode, other modules trying to do role-switch on the same
>> port have to first check the EUD status by calling this notifier chain and based
>> on the status proceed or block their role-switching step. The modules can call
>> the notifier through the call eud_notifier_call_chain and pass their own
>> role switch state as the argument. This chain will also be able to handle the
>> scenario of multiple modules switching roles on the same port since this can
>> create a priority and ordering among them for conflict resolution.
> 
> You are adding a new api that no one is actually using, so why would we
> accept this at all?
> 
> And how can we actually review it without any real users?

Ack. The next version of this will be posted along with the usage
of these apis for ordering the role-switch.

> 
> thanks,
> 
> greg k-h
Souradeep Chowdhury July 13, 2023, 8:26 a.m. UTC | #4
On 7/12/2023 10:13 PM, Konrad Dybcio wrote:
> 
> 
> On 12.07.2023 10:22, Souradeep Chowdhury wrote:
>> This patch series adds the notifier chain to the Embedded USB Debug(EUD) driver.
>> The notifier chain is used to check the role switch status of EUD. Since EUD can
>> function only in device mode, other modules trying to do role-switch on the same
>> port have to first check the EUD status by calling this notifier chain and based
>> on the status proceed or block their role-switching step. The modules can call
>> the notifier through the call eud_notifier_call_chain and pass their own
>> role switch state as the argument. This chain will also be able to handle the
>> scenario of multiple modules switching roles on the same port since this can
>> create a priority and ordering among them for conflict resolution.
>>
>> Souradeep Chowdhury (3):
>>    usb: misc: Add the interface for notifier call for Embedded USB
>>      Debugger(EUD)
>>    usb: misc: Add notifier call chain to Embedded USB Debug(EUD) driver
>>    MAINTAINERS: Add the header file entry for Embedded USB debugger(EUD)
> Please actually CC all maintainers, as present in the MAINTAINERS file..
> 
> Consider using b4:
> 
> https://b4.docs.kernel.org/en/latest/index.html

Ack

> 
> Konrad
>>
>>   MAINTAINERS                          |  1 +
>>   drivers/usb/misc/qcom_eud.c          | 52 ++++++++++++++++++++++++++--
>>   drivers/usb/misc/qcom_eud_notifier.h | 10 ++++++
>>   3 files changed, 61 insertions(+), 2 deletions(-)
>>   create mode 100644 drivers/usb/misc/qcom_eud_notifier.h
>>
>> --
>> 2.17.1
>>
Greg Kroah-Hartman July 13, 2023, 1:52 p.m. UTC | #5
On Thu, Jul 13, 2023 at 01:55:34PM +0530, Souradeep Chowdhury wrote:
> 
> 
> On 7/12/2023 10:10 PM, Greg KH wrote:
> > On Wed, Jul 12, 2023 at 01:52:37PM +0530, Souradeep Chowdhury wrote:
> > > This patch series adds the notifier chain to the Embedded USB Debug(EUD) driver.
> > > The notifier chain is used to check the role switch status of EUD. Since EUD can
> > > function only in device mode, other modules trying to do role-switch on the same
> > > port have to first check the EUD status by calling this notifier chain and based
> > > on the status proceed or block their role-switching step. The modules can call
> > > the notifier through the call eud_notifier_call_chain and pass their own
> > > role switch state as the argument. This chain will also be able to handle the
> > > scenario of multiple modules switching roles on the same port since this can
> > > create a priority and ordering among them for conflict resolution.
> > 
> > You are adding a new api that no one is actually using, so why would we
> > accept this at all?
> > 
> > And how can we actually review it without any real users?
> 
> Ack. The next version of this will be posted along with the usage
> of these apis for ordering the role-switch.

Also note that I hate notifier call chains, so you better have a very
good reason for why they are required and can't be done in any other
normal way instead :)

good luck!

greg k-h