On Tue, Sep 12, 2023 at 05:48:44PM +0800, Jinjian Song wrote: > From: Jinjian Song <jinjian.song@xxxxxxxxxxx> > > Adds support for t7xx wwan device firmware flashing & coredump collection > using devlink. > > Provides sysfs attribute on user space to query the event from modem > about flashing/coredump/reset. > > Base on the v5 patch version of follow series: > 'net: wwan: t7xx: fw flashing & coredump support' > (https://patchwork.kernel.org/project/netdevbpf/patch/fc8bbb0b66a5ff3a489ea9857d79b374508090ef.1674307425.git.m.chetan.kumar@xxxxxxxxxxxxxxx/) > > Signed-off-by: Jinjian Song <jinjian.song@xxxxxxxxxxx> Hi Jinjian Song, some minor feedback from my side. ... > diff --git a/drivers/net/wwan/t7xx/t7xx_pci.c b/drivers/net/wwan/t7xx/t7xx_pci.c ... > +static ssize_t t7xx_event_show(struct device *dev, struct device_attribute *attr, > + char *buf) > +{ > + enum t7xx_event event = T7XX_UNKNOWN; > + struct pci_dev *pdev; > + struct t7xx_pci_dev *t7xx_dev; Please arrange local variables in networking code in reverse xmas tree order - longest line to shortest. https://github.com/ecree-solarflare/xmastree can be helpful here. ... > diff --git a/drivers/net/wwan/t7xx/t7xx_port_flash_dump.c b/drivers/net/wwan/t7xx/t7xx_port_flash_dump.c ... > @@ -361,6 +367,10 @@ static int t7xx_devlink_flash_update(struct devlink *devlink, > clear_bit(T7XX_FLASH_STATUS, &flash_dump->status); > > err_out: > + if (ret) > + atomic_set(&port->t7xx_dev->event, T7XX_FLASH_FAILURE); > + else > + atomic_set(&port->t7xx_dev->event, T7XX_FLASH_SUCCESS); If the lines immediately above are reached as the result of jumping to err_out, then port is not initialised. > return ret; > } ...
