Re: [PATCH net-next] net: airoha: Add missing filed to ppe_mbox_data struct

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

> On Tue, Apr 15, 2025 at 09:27:21AM +0200, Lorenzo Bianconi wrote:
> > The official Airoha EN7581 firmware requires adding max_packet filed in
> > ppe_mbox_data struct while the unofficial one used to develop the Airoha
> > EN7581 flowtable offload does not require this field. This patch fixes
> > just a theoretical bug since the Airoha EN7581 firmware is not posted to
> > linux-firware or other repositories (e.g. OpenWrt) yet.
> > 
> > Fixes: 23290c7bc190d ("net: airoha: Introduce Airoha NPU support")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/net/ethernet/airoha/airoha_npu.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/net/ethernet/airoha/airoha_npu.c b/drivers/net/ethernet/airoha/airoha_npu.c
> > index 7a5710f9ccf6a4a4f555ab63d67cb6b318de9b52..16201b5ce9f27866896226c3611b4a154d19bc2c 100644
> > --- a/drivers/net/ethernet/airoha/airoha_npu.c
> > +++ b/drivers/net/ethernet/airoha/airoha_npu.c
> > @@ -104,6 +104,7 @@ struct ppe_mbox_data {
> >  			u8 xpon_hal_api;
> >  			u8 wan_xsi;
> >  			u8 ct_joyme4;
> > +			u8 max_packet;
> >  			int ppe_type;
> >  			int wan_mode;
> >  			int wan_sel;
> 
> Hi Lorenzo,
> 
> I'm a little confused by this.
> 
> As I understand it ppe_mbox_data is sent as the data of a mailbox message
> send to the device.  But by adding the max_packet field the layout is
> changed. The size of the structure changes. And perhaps more importantly
> the offset of fields after max_packet, e.g.  wan_mode, change.
> 
> Looking at how this is used, f.e. in the following code, I'm unclear on
> how this change is backwards compatible.

you are right Simon, this change is not backwards compatible but the fw is
not publicly available yet and the official fw version will use this new layout
(the previous one was just a private version I used to develop the driver).
Can we use this simple approach or do you think we should differentiate the two
firmware version in some way? (even if the previous one will never be used).

Regards,
Lorenzo

> 
> static int airoha_npu_ppe_init(struct airoha_npu *npu)
> {
>         struct ppe_mbox_data ppe_data = {
>                 .func_type = NPU_OP_SET,
>                 .func_id = PPE_FUNC_SET_WAIT_HWNAT_INIT,
>                 .init_info = {
>                         .ppe_type = PPE_TYPE_L2B_IPV4_IPV6,
>                         .wan_mode = QDMA_WAN_ETHER,
>                 },
>         };
> 
>         return airoha_npu_send_msg(npu, NPU_FUNC_PPE, &ppe_data,
>                                    sizeof(struct ppe_mbox_data));
> }

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]