| Message ID | 3f904c291f3eed06223dd8d494028e0d49df6f10.1636711522.git.christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | mei: Remove some dead code | expand |
> On 12 Nov 2021, at 11:06, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote: > > 'generated' is known to be true here, so "true || whatever" will still be > true. > > So, remove some dead code. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > This is also likely that a bug is lurking here. > > Maybe, the following was expected: > - generated = generated || > + generated = > (hisr & HISR_INT_STS_MSK) || > (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); > > ? I concur about your analysis, but I do not know the intent here. Håkon > --- > drivers/misc/mei/hw-txe.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c > index a4e854b9b9e6..00652c137cc7 100644 > --- a/drivers/misc/mei/hw-txe.c > +++ b/drivers/misc/mei/hw-txe.c > @@ -994,11 +994,7 @@ static bool mei_txe_check_and_ack_intrs(struct mei_device *dev, bool do_ack) > hhisr &= ~IPC_HHIER_SEC; > } > > - generated = generated || > - (hisr & HISR_INT_STS_MSK) || > - (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); > - > - if (generated && do_ack) { > + if (do_ack) { > /* Save the interrupt causes */ > hw->intr_cause |= hisr & HISR_INT_STS_MSK; > if (ipc_isr & SEC_IPC_HOST_INT_STATUS_IN_RDY) > -- > 2.30.2 >
> > > > On 12 Nov 2021, at 11:06, Christophe JAILLET > <christophe.jaillet@wanadoo.fr> wrote: > > > > 'generated' is known to be true here, so "true || whatever" will still > > be true. > > > > So, remove some dead code. > > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > --- > > This is also likely that a bug is lurking here. > > > > Maybe, the following was expected: > > - generated = generated || > > + generated = > > (hisr & HISR_INT_STS_MSK) || > > (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); > > > > ? > > I concur about your analysis, but I do not know the intent here. Your fix is okay, I can ack that patch. Thanks Tomas > > > Håkon > > > --- > > drivers/misc/mei/hw-txe.c | 6 +----- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c > > index a4e854b9b9e6..00652c137cc7 100644 > > --- a/drivers/misc/mei/hw-txe.c > > +++ b/drivers/misc/mei/hw-txe.c > > @@ -994,11 +994,7 @@ static bool mei_txe_check_and_ack_intrs(struct > mei_device *dev, bool do_ack) > > hhisr &= ~IPC_HHIER_SEC; > > } > > > > - generated = generated || > > - (hisr & HISR_INT_STS_MSK) || > > - (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); > > - > > - if (generated && do_ack) { > > + if (do_ack) { > > /* Save the interrupt causes */ > > hw->intr_cause |= hisr & HISR_INT_STS_MSK; > > if (ipc_isr & SEC_IPC_HOST_INT_STATUS_IN_RDY) > > -- > > 2.30.2 > >
On Sun, Nov 28, 2021 at 11:12:33AM +0000, Winkler, Tomas wrote: > > > > > > > > On 12 Nov 2021, at 11:06, Christophe JAILLET > > <christophe.jaillet@wanadoo.fr> wrote: > > > > > > 'generated' is known to be true here, so "true || whatever" will still > > > be true. > > > > > > So, remove some dead code. > > > > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > --- > > > This is also likely that a bug is lurking here. > > > > > > Maybe, the following was expected: > > > - generated = generated || > > > + generated = > > > (hisr & HISR_INT_STS_MSK) || > > > (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); > > > > > > ? > > > > I concur about your analysis, but I do not know the intent here. > Your fix is okay, I can ack that patch. Is that an ack of this patch? If so, please provide that... thanks, greg k-h
> On Sun, Nov 28, 2021 at 11:12:33AM +0000, Winkler, Tomas wrote: > > > > > > > > > > > > On 12 Nov 2021, at 11:06, Christophe JAILLET > > > <christophe.jaillet@wanadoo.fr> wrote: > > > > > > > > 'generated' is known to be true here, so "true || whatever" will > > > > still be true. > > > > > > > > So, remove some dead code. > > > > > > > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > > > --- > > > > This is also likely that a bug is lurking here. > > > > > > > > Maybe, the following was expected: > > > > - generated = generated || > > > > + generated = > > > > (hisr & HISR_INT_STS_MSK) || > > > > (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); > > > > > > > > ? > > > > > > I concur about your analysis, but I do not know the intent here. > > Your fix is okay, I can ack that patch. > > Is that an ack of this patch? If so, please provide that... Acked-by: Tomas Winkler <tomas.winkler@intel.com> Thanks Tomas
diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c index a4e854b9b9e6..00652c137cc7 100644 --- a/drivers/misc/mei/hw-txe.c +++ b/drivers/misc/mei/hw-txe.c @@ -994,11 +994,7 @@ static bool mei_txe_check_and_ack_intrs(struct mei_device *dev, bool do_ack) hhisr &= ~IPC_HHIER_SEC; } - generated = generated || - (hisr & HISR_INT_STS_MSK) || - (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); - - if (generated && do_ack) { + if (do_ack) { /* Save the interrupt causes */ hw->intr_cause |= hisr & HISR_INT_STS_MSK; if (ipc_isr & SEC_IPC_HOST_INT_STATUS_IN_RDY)
'generated' is known to be true here, so "true || whatever" will still be true. So, remove some dead code. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- This is also likely that a bug is lurking here. Maybe, the following was expected: - generated = generated || + generated = (hisr & HISR_INT_STS_MSK) || (ipc_isr & SEC_IPC_HOST_INT_STATUS_PENDING); ? --- drivers/misc/mei/hw-txe.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)