On Thu, Oct 13, 2022 at 06:25:54PM +0200, Lorenzo Bianconi wrote:
> > I would like to revisit this question. Last time I complained about
> > this Johannes responded but he misread what mt76_mcu_send_and_get_msg()
> > does. I have looked at it as well and I also cannot explain what is
> > going on in that function.
> >
> > I have looked at the callers and my first instinct is that maybe this
> > is dead stub code? But then when I look at mt76x02u_mcu_send_msg() I
> > think "No, this is not stub code. This should be returning the newly
> > allocated skb to the caller."
> >
> > But then I think, surely at some point someone tested this code??? It
> > must be stub code.
> >
> > Could we get some clarity on this?
>
> for mt76x2 and mt76x0 we do not care of ret_skb (in fact we do not run
> mt76_mcu_send_and_get_msg() directly but we rely on mt76_mcu_send_msg()).
> For mt7921 we set mcu_skb_send_msg function pointer and not mcu_send_msg.
Ah thanks... It's easy enough to silence the warning in Smatch but I
was never sure if it wasn't a bug.
> Moreover mt7921_mcu_get_eeprom() has been remove a while back.
> Am I missing something?
There are 12 callers for mt76_mcu_send_and_get_msg() and 11 of them
assume that the "ret_skb" is initialized (i.e. they assume that
the ->mcu_send_msg op is not used) so I get 11 Smatch warnings from
this...
Why not just do something like below? It moves the ->mcu_send_msg()
call to the only place where it won't cause a crash.
regards,
dan carpenter
diff --git a/drivers/net/wireless/mediatek/mt76/mcu.c b/drivers/net/wireless/mediatek/mt76/mcu.c
index a8cafa39a56d..6bf0b7d8daee 100644
--- a/drivers/net/wireless/mediatek/mt76/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mcu.c
@@ -58,9 +58,6 @@ int mt76_mcu_send_and_get_msg(struct mt76_dev *dev, int cmd, const void *data,
{
struct sk_buff *skb;
- if (dev->mcu_ops->mcu_send_msg)
- return dev->mcu_ops->mcu_send_msg(dev, cmd, data, len, wait_resp);
-
skb = mt76_mcu_msg_alloc(dev, data, len);
if (!skb)
return -ENOMEM;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 87db9498dea4..99f931c08da9 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -1383,6 +1383,9 @@ static inline int
mt76_mcu_send_msg(struct mt76_dev *dev, int cmd, const void *data, int len,
bool wait_resp)
{
+ if (dev->mcu_ops->mcu_send_msg)
+ return dev->mcu_ops->mcu_send_msg(dev, cmd, data, len, wait_resp);
+
return mt76_mcu_send_and_get_msg(dev, cmd, data, len, wait_resp, NULL);
}
