Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   =?ISO-8859-1?Q?J=E9r=F4me?= Pouiller <jerome.pouiller@silabs.com>
To:     Kalle Valo <kvalo@kernel.org>
Cc:     linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "David S . Miller" <davem@davemloft.net>,
        devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
        linux-mmc@vger.kernel.org,
        Pali =?ISO-8859-1?Q?Roh=E1r?= <pali@kernel.org>,
        Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH v9 05/24] wfx: add main.c/main.h
Date:   Thu, 10 Feb 2022 15:41:39 +0100
Message-ID: <4055223.VTxhiZFAix@pc-42>
Organization: Silicon Labs
In-Reply-To: <87r18a3irb.fsf@kernel.org>
References: <20220111171424.862764-1-Jerome.Pouiller@silabs.com> <2898137.rlL8Y2EFai@pc-42> <87r18a3irb.fsf@kernel.org>
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?5FgQyJ+2zUadajL7be1kwaFhBV+0RuXRNJSEACtz4mWQztTt6z8AskBgQV?=
 =?iso-8859-1?Q?C28nTp0F/pXtLA9WWORmkFmgKJ8ykSdn6VtWYxBnlxqlPmIoHxUXY5DmPz?=
 =?iso-8859-1?Q?qwId2kD3BrzvYSavaR0OjOT+MPK3Jt5J/7s7Xe9FIG+xX7VQvyJjKUwwcw?=
 =?iso-8859-1?Q?iM21MRL3eWlv0kJPkAvojX76TfyH5aCZUVoWxCwxees3BD+QrmFVWZ0E0a?=
 =?iso-8859-1?Q?iFF5dSDZYwfjjDiAnkKD8cdg1fJMK1DS6SA6yeEUwU4LfBhh6nUNORGwXZ?=
 =?iso-8859-1?Q?JUhRespPiDRkfmDitYyj4mnI7cZYj/4VKyjYgvNkNHITpJitbMkjXKYkKx?=
 =?iso-8859-1?Q?qJJlR8tsGIKafpUT/3t/s8E2aVbN2MNF5k9Sk0NAA1koDLLRZJop8rbbxE?=
 =?iso-8859-1?Q?Zu9I1vgESZJKEfV99seE83P8t0OGncAgSTSCNsc6hkUGo8vhtTm4jwjyTp?=
 =?iso-8859-1?Q?Awauauo3HmhfbTD4lahG36fkFBewE6ExHk5gUwrxJRWHryMsZqw+OvxAKj?=
 =?iso-8859-1?Q?DGG0e8FPJB65K9iiUSJc+iXEvEbCgQ4vqZScdHtqq9DsZ6VPQmcLP3IOOp?=
 =?iso-8859-1?Q?rVQHk1KGTAbgTKwkoV0cgB6LSBWj3oLPYbWAMkDxESF5qSb3nV7mthD4yd?=
 =?iso-8859-1?Q?9903LWKR1IV6Ex0P9Xia4NkClYC/oNl3tjvgLS/9SqAG0PZ0NjsYuiIiZE?=
 =?iso-8859-1?Q?80cDbMNsnlZgYqNHP2s2nXGuu/NfOBNv6+QZ5jaJuYBxbod/UZKA50Abe/?=
 =?iso-8859-1?Q?2sgsOF+gsfFtg6CIA8jBHFATt+v9SLzJFyJryD9kN/rTvUpGePlb5pBiQQ?=
 =?iso-8859-1?Q?PssyM4UyPPeWmxlLSTw80ko++etDWf67FAYIe0IBJJb3d0trMRc7H0Plx/?=
 =?iso-8859-1?Q?jRMOSnf225ovYhhkA2z3sMJHKg2Hy3cQHuIEmJCgXjlJaKpci4FkKyidQ5?=
 =?iso-8859-1?Q?c+eQ9xbgULM4Idv8E4DvTfnlM1po6yaozJun5v/lOGU6aBO8Hh/n12hwO+?=
 =?iso-8859-1?Q?+pyi/do+FYspmj5lV6stgpwcHH9MRK3V/YJwsH5GC4SpgpUfiML8yILCMp?=
 =?iso-8859-1?Q?ZJUkGm0JCj+DXs6pDAKGel/0pWNMna2OOOSMK4G3SnNv184/gyz55c6Mcq?=
 =?iso-8859-1?Q?jCyjHEuhA5ha7oNrEsvTUL9xiXnLJQWLQItB+oM3E0bzXzzDsV68kaqE1J?=
 =?iso-8859-1?Q?6YYLDcAwOTK12dW2voRg8TDCJsv9QJKYpnLNrjHmzZpLLDgniPpnk6gNyJ?=
 =?iso-8859-1?Q?eph3bjwfQ183enPkyJ7cnWg3lWgwrkqoFzMNVBBH90hQYHGiRuxjPkXl6Q?=
 =?iso-8859-1?Q?iy/Ccf8yVI7XiawiUo5cz2zEukFx1IJToeiwIqks/nmFwfc//LwDwtOGAm?=
 =?iso-8859-1?Q?9cl8lhdg/a8bWJ297JDVbUR+QcHTsHzFmJVFjv1DyZL8fex8Ek1RMGTB/H?=
 =?iso-8859-1?Q?PF/tvCYbhk0qq08pIKB025+63ZPRGj1q7bggi5C/vt9XyYK7ViJRzImU6d?=
 =?iso-8859-1?Q?ktamU5kUeO0/yI3ty8e1y/xvQrBjwj4qBw5Ww4U9wVYye+iMn+d0IV/9VW?=
 =?iso-8859-1?Q?srNi0bKEtN8/9JNUrsdQmY/7iyAt/zoeZM1uSBS9OVvZlROvGzqbDFj3X+?=
 =?iso-8859-1?Q?EOCosD2hTSs+5nueVkyP/TxzcCehjmq5lM/ebSq7F7VbOSz/xuYTQ7QtJX?=
 =?iso-8859-1?Q?ZQ8kCSzoMmEugYaUlvQ=3D?=
X-OriginatorOrg: silabs.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c4471b21-962e-4c5c-aa8d-08d9eca37793
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB5657.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Feb 2022 14:41:48.2295
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 54dbd822-5231-4b20-944d-6f4abcd541fb
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: GHaOPhpxYGpiShinA9Lyo11OJ4mYIpLCDYAy6U4NCKI0HVH2nF4vPcn2ORsqK8hMhS+to9hy7/egUuJrdxO5XQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR11MB3167
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

On Thursday 10 February 2022 15:20:56 CET Kalle Valo wrote:
>=20
> J=E9r=F4me Pouiller <jerome.pouiller@silabs.com> writes:
>=20
> > Hi Kalle,
> >
> > On Tuesday 11 January 2022 18:14:05 CET Jerome Pouiller wrote:
> >> From: J=E9r=F4me Pouiller <jerome.pouiller@silabs.com>
> >>
> >> Signed-off-by: J=E9r=F4me Pouiller <jerome.pouiller@silabs.com>
> >> ---
> >>  drivers/net/wireless/silabs/wfx/main.c | 485 ++++++++++++++++++++++++=
+
> >>  drivers/net/wireless/silabs/wfx/main.h |  42 +++
> >>  2 files changed, 527 insertions(+)
> >>  create mode 100644 drivers/net/wireless/silabs/wfx/main.c
> >>  create mode 100644 drivers/net/wireless/silabs/wfx/main.h
> >>
> > [...]
> >> +/* The device needs data about the antenna configuration. This inform=
ation in
> >> + * provided by PDS (Platform Data Set, this is the wording used in WF=
200
> >> + * documentation) files. For hardware integrators, the full process t=
o create
> >> + * PDS files is described here:
> >> + *   https:github.com/SiliconLabs/wfx-firmware/blob/master/PDS/README=
.md
> >> + *
> >> + * The PDS file is an array of Time-Length-Value structs.
> >> + */
> >> + int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len)
> >> +{
> >> +    int ret, chunk_type, chunk_len, chunk_num =3D 0;
> >> +
> >> +    if (*buf =3D=3D '{') {
> >> +            dev_err(wdev->dev, "PDS: malformed file (legacy format?)\=
n");
> >> +            return -EINVAL;
> >> +    }
> >> +    while (len > 0) {
> >> +            chunk_type =3D get_unaligned_le16(buf + 0);
> >> +            chunk_len =3D get_unaligned_le16(buf + 2);
> >> +            if (chunk_len > len) {
> >> +                    dev_err(wdev->dev, "PDS:%d: corrupted file\n", ch=
unk_num);
> >> +                    return -EINVAL;
> >> +            }
> >> +            if (chunk_type !=3D WFX_PDS_TLV_TYPE) {
> >> +                    dev_info(wdev->dev, "PDS:%d: skip unknown data\n"=
, chunk_num);
> >> +                    goto next;
> >> +            }
> >> +            if (chunk_len > WFX_PDS_MAX_CHUNK_SIZE)
> >> + dev_warn(wdev->dev, "PDS:%d: unexpectly large chunk\n",
> >> chunk_num);
> >> +            if (buf[4] !=3D '{' || buf[chunk_len - 1] !=3D '}')
> >> + dev_warn(wdev->dev, "PDS:%d: unexpected content\n", chunk_num);
> >> +
> >> +            ret =3D wfx_hif_configuration(wdev, buf + 4, chunk_len - =
4);
> >> +            if (ret > 0) {
> >> + dev_err(wdev->dev, "PDS:%d: invalid data (unsupported
> >> options?)\n",
> >> +                            chunk_num);
> >> +                    return -EINVAL;
> >> +            }
> >> +            if (ret =3D=3D -ETIMEDOUT) {
> >> + dev_err(wdev->dev, "PDS:%d: chip didn't reply (corrupted
> >> file?)\n",
> >> +                            chunk_num);
> >> +                    return ret;
> >> +            }
> >> +            if (ret) {
> >> + dev_err(wdev->dev, "PDS:%d: chip returned an unknown error\n",
> >> chunk_num);
> >> +                    return -EIO;
> >> +            }
> >> +next:
> >> +            chunk_num++;
> >> +            len -=3D chunk_len;
> >> +            buf +=3D chunk_len;
> >> +    }
> >> +    return 0;
> >> +}
> >
> > Kalle, is this function what you expected? If it is right for you, I am
> > going to send it to the staging tree.
>=20
> Looks better, but I don't get why '{' and '}' are still needed. Ah, does
> the firmware require to have them?

Indeed. If '{' and '}' are not present, I guarantee the firmware will retur=
n
an error (or assert). However, I am more confident in the driver than in th=
e
firmware to report errors to the user.

If there is no other comment, I am going to:
  - submit this change to the staging tree
  - publish the tool that generate this new format
  - submit the PDS files referenced in bus_{sdio,spi}.c to linux-firmware
  - send the v10 of this PR


--=20
J=E9r=F4me Pouiller