On Thu, Jan 19, 2023 at 08:16:52AM +0000, Ki-Seok Jo wrote:
> > > + if (!(sma1303->amp_power_status)) {
> > > + dev_info(component->dev, "%s : %s\n",
> > > + __func__, "Already AMP Shutdown");
> > > + return ret;
> > > + }
> > > +
> > > + cancel_delayed_work_sync(&sma1303->check_fault_work);
> > > +
> > > + msleep(55);
> > That sleep looks odd - what are we delaying after?
> It need for IC(Amp) issue.
Right, but what is the issue? It's not clear what event we're
delaying for so it's not clear it'll work properly.
> > > +static void sma1303_check_fault_worker(struct work_struct *work) {
> > > + struct sma1303_priv *sma1303 =
> > > + container_of(work, struct sma1303_priv, check_fault_work.work);
> > > + int ret = 0;
> > > + unsigned int over_temp, ocp_val, uvlo_val;
> > > +
> > > + mutex_lock(&sma1303->lock);
> > It looks like this mutex is only taken in this function, is it needed?
> This function is in workqueue. So, I think it can be done at the same time.
A given work_struct should only be schedulable once.
Attachment:
signature.asc
Description: PGP signature
