On Tue, 16 Jan 2024 14:18:04 +0100
Andrew Lunn <andrew@xxxxxxx> wrote:
> >
> > > > +static int pd692x0_fw_get_next_line(const u8 *data,
> > > > + char *line, size_t size)
> > > > +{
> > > > + size_t line_size;
> > > > + int i;
> > > > +
> > > > + line_size = min_t(size_t, size,
> > > > (size_t)PD692X0_FW_LINE_MAX_SZ); +
> > > > + memset(line, 0, PD692X0_FW_LINE_MAX_SZ);
> > > > + for (i = 0; i < line_size - 1; i++) {
> > > > + if (*data == '\r' && *(data + 1) == '\n') {
> > > > + line[i] = '\r';
> > > > + line[i + 1] = '\n';
> > > > + return i + 2;
> > > > + }
> > >
> > > Does the Vendor Documentation indicate Windoze line endings will
> > > always be used? Motorola SREC allow both Windows or rest of the world
> > > line endings to be used.
> >
> > All the firmware lines end with "\r\n" but indeed it is not specifically
> > written that the firmware content would follow this. IMHO it is implicit
> > that it would be the case as all i2c messages use this line termination.
> > Do you prefer that I add support to the world line endings possibility?
>
> No need, just hack an SREC file, and test the parser does not explode
> with an opps, and you get an sensible error message about the firmware
> being corrupt. I would not be too surprised if there are some mail
> systems still out there which might convert the line ending.
Ok I will do so.
>
> > > > +static enum fw_upload_err pd692x0_fw_poll_complete(struct fw_upload
> > > > *fwl) +{
> > > > + struct pd692x0_priv *priv = fwl->dd_handle;
> > > > + const struct i2c_client *client = priv->client;
> > > > + struct pd692x0_msg_ver ver;
> > > > + int ret;
> > > > +
> > > > + priv->fw_state = PD692X0_FW_COMPLETE;
> > > > +
> > > > + ret = pd692x0_fw_reset(client);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + ver = pd692x0_get_sw_version(priv);
> > > > + if (ver.maj_sw_ver != PD692X0_FW_MAJ_VER) {
> > >
> > > That is probably too strong a condition. You need to allow firmware
> > > upgrades, etc. Does it need to be an exact match, or would < be
> > > enough?
> >
> > The major version is not compatible with the last one, the i2c messages
> > content changed. I supposed a change in major version would imply a change
> > in the i2c messages content and would need a driver update that's why I
> > used this strong condition.
>
> Do you know the next major version will change the message contents?
No.
> Is this documented somewhere? If so add a comment. Otherwise, i would
> allow higher major versions. When the vendor breaks backwards
> compatibility, its going to need code changes anyway, and at that
> point the test can be made more strict.
>
> We try to make vendors not make firmware ABI breaking changes, and we
> have pushed back against a number of vendors who do. So i think its
> best we assume they won't break the ABI.
Alright, thanks!
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
