lkml.org 
[lkml]   [2021]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH v7 05/24] wfx: add main.c/main.h
    Date
    On Tuesday 5 October 2021 07:56:53 CEST Kalle Valo wrote:
    > Jérôme Pouiller <jerome.pouiller@silabs.com> writes:
    >
    > > On Friday 1 October 2021 11:22:08 CEST Kalle Valo wrote:
    > >> CAUTION: This email originated from outside of the organization. Do
    > >> not click links or open attachments unless you recognize the sender
    > >> and know the content is safe.
    > >>
    > >>
    > >> Jerome Pouiller <Jerome.Pouiller@silabs.com> writes:
    > >>
    > >> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
    > >> >
    > >> > Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
    > >>
    > >> [...]
    > >>
    > >> > +/* The device needs data about the antenna configuration. This information in
    > >> > + * provided by PDS (Platform Data Set, this is the wording used in WF200
    > >> > + * documentation) files. For hardware integrators, the full process to create
    > >> > + * PDS files is described here:
    > >> > + * https:github.com/SiliconLabs/wfx-firmware/blob/master/PDS/README.md
    > >> > + *
    > >> > + * So this function aims to send PDS to the device. However, the PDS file is
    > >> > + * often bigger than Rx buffers of the chip, so it has to be sent in multiple
    > >> > + * parts.
    > >> > + *
    > >> > + * In add, the PDS data cannot be split anywhere. The PDS files contains tree
    > >> > + * structures. Braces are used to enter/leave a level of the tree (in a JSON
    > >> > + * fashion). PDS files can only been split between root nodes.
    > >> > + */
    > >> > +int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len)
    > >> > +{
    > >> > + int ret;
    > >> > + int start, brace_level, i;
    > >> > +
    > >> > + start = 0;
    > >> > + brace_level = 0;
    > >> > + if (buf[0] != '{') {
    > >> > + dev_err(wdev->dev, "valid PDS start with '{'. Did you forget to
    > >> > compress it?\n");
    > >> > + return -EINVAL;
    > >> > + }
    > >> > + for (i = 1; i < len - 1; i++) {
    > >> > + if (buf[i] == '{')
    > >> > + brace_level++;
    > >> > + if (buf[i] == '}')
    > >> > + brace_level--;
    > >> > + if (buf[i] == '}' && !brace_level) {
    > >> > + i++;
    > >> > + if (i - start + 1 > WFX_PDS_MAX_SIZE)
    > >> > + return -EFBIG;
    > >> > + buf[start] = '{';
    > >> > + buf[i] = 0;
    > >> > + dev_dbg(wdev->dev, "send PDS '%s}'\n", buf + start);
    > >> > + buf[i] = '}';
    > >> > + ret = hif_configuration(wdev, buf + start,
    > >> > + i - start + 1);
    > >> > + if (ret > 0) {
    > >> > + dev_err(wdev->dev, "PDS bytes %d to %d: invalid data (unsupported
    > >> > options?)\n",
    > >> > + start, i);
    > >> > + return -EINVAL;
    > >> > + }
    > >> > + if (ret == -ETIMEDOUT) {
    > >> > + dev_err(wdev->dev, "PDS bytes %d to %d: chip didn't reply (corrupted
    > >> > file?)\n",
    > >> > + start, i);
    > >> > + return ret;
    > >> > + }
    > >> > + if (ret) {
    > >> > + dev_err(wdev->dev, "PDS bytes %d to %d: chip returned an unknown
    > >> > error\n",
    > >> > + start, i);
    > >> > + return -EIO;
    > >> > + }
    > >> > + buf[i] = ',';
    > >> > + start = i;
    > >> > + }
    > >> > + }
    > >> > + return 0;
    > >> > +}
    > >>
    > >> I'm not really fond of having this kind of ASCII based parser in the
    > >> kernel. Do you have an example compressed file somewhere?
    > >>
    > >> Does the device still work without these PDS files? I ask because my
    > >> suggestion is to remove this part altogether and revisit after the
    > >> initial driver is moved to drivers/net/wireless. A lot simpler to review
    > >> complex features separately.
    > >
    > > Do you want I remove this function from this patch and place it a new
    > > patch at the end of this series?
    >
    > I don't understand, how that would help? The problem here is the file
    > format and that's what we should try to fix.

    It was just to be able to review this function separately. Nevermind.


    --
    Jérôme Pouiller


    \
     
     \ /
      Last update: 2021-10-05 10:13    [W:4.284 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site