| Message ID | 20210511022224.1309077-2-bryan.odonoghue@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: typec: tcpm: Fix logic and documentation of tcpc_dev->init | expand |
On 5/10/21 7:22 PM, Bryan O'Donoghue wrote: > The tcpc_dev structure lists a number of callbacks as required when > implementing a TCPM driver. tcpc_dev->init() is not listed as required. > > Currently tcpc_dev->init() is called irrespective of whether or not the > callback is set. Let's conditionally call init() as with other non-required > callbacks such as get_current_limit() or set_current_limit(). > > Fixes: f0690a25a140b ("staging: typec: USB Type-C Port Manager (tcpm)") > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> Are we going to see a driver with no init function ? If not, I would suggest to make the call officially mandatory. Guenter > --- > drivers/usb/typec/tcpm/tcpm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c > index c4fdc00a3bc8..355067e6d420 100644 > --- a/drivers/usb/typec/tcpm/tcpm.c > +++ b/drivers/usb/typec/tcpm/tcpm.c > @@ -5657,7 +5657,8 @@ static void tcpm_init(struct tcpm_port *port) > { > enum typec_cc_status cc1, cc2; > > - port->tcpc->init(port->tcpc); > + if (port->tcpc->init) > + port->tcpc->init(port->tcpc); > > tcpm_reset_port(port); > >
On 11/05/2021 03:44, Guenter Roeck wrote: > Are we going to see a driver with no init function ? > If not, I would suggest to make the call officially mandatory. > > Guenter again in plaintext.. I have some tpcm code that doesn't need to put anything into the init function which is how I found this. Its very much up to yourself on making init() mandatory though. I'm just as happy to redo the patch and add a check to the add port routine
On 5/11/21 2:21 AM, Bryan O'Donoghue wrote: > On 11/05/2021 03:44, Guenter Roeck wrote: >> Are we going to see a driver with no init function ? >> If not, I would suggest to make the call officially mandatory. >> >> Guenter > > again in plaintext.. > > I have some tpcm code that doesn't need to put anything into the init function which is how I found this. > > Its very much up to yourself on making init() mandatory though. I'm just as happy to redo the patch and add a check to the add port routine No need, but maybe submit the code that doesn't need it together with the change making it optional so we can see the actual use case. Thanks, Guenter
On 11/05/2021 20:03, Guenter Roeck wrote: > No need, but maybe submit the code that doesn't need it together > with the change making it optional so we can see the actual use case. Fair enough
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index c4fdc00a3bc8..355067e6d420 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -5657,7 +5657,8 @@ static void tcpm_init(struct tcpm_port *port) { enum typec_cc_status cc1, cc2; - port->tcpc->init(port->tcpc); + if (port->tcpc->init) + port->tcpc->init(port->tcpc); tcpm_reset_port(port);
The tcpc_dev structure lists a number of callbacks as required when implementing a TCPM driver. tcpc_dev->init() is not listed as required. Currently tcpc_dev->init() is called irrespective of whether or not the callback is set. Let's conditionally call init() as with other non-required callbacks such as get_current_limit() or set_current_limit(). Fixes: f0690a25a140b ("staging: typec: USB Type-C Port Manager (tcpm)") Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- drivers/usb/typec/tcpm/tcpm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)