Re: [RFC PATCH v3] can: virtio: Initial virtio CAN driver.

[Vincent Mailhol]

Hi Simon,

On Fri. 12 May 2023 at 00:45, Simon Horman <simon.horman@xxxxxxxxxxxx> wrote:
> On Thu, May 11, 2023 at 05:14:44PM +0200, Mikhail Golubev-Ciuchea wrote:

[...]

> > +static u8 virtio_can_send_ctrl_msg(struct net_device *ndev, u16 msg_type)
> > +{
> > +     struct virtio_can_priv *priv = netdev_priv(ndev);
> > +     struct device *dev = &priv->vdev->dev;
> > +     struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_CONTROL];
> > +     struct scatterlist sg_out[1];
> > +     struct scatterlist sg_in[1];
> > +     struct scatterlist *sgs[2];
> > +     int err;
> > +     unsigned int len;
>
> nit: For networking code please arrange local variables in reverse xmas
>      tree order - longest line to shortest.

Sorry for my curiosity, but where is it documented that the networking
code is using reverse christmas tree style?

I already inquired in the past here:

  https://lore.kernel.org/linux-can/CAMZ6Rq+zsC4F-mNhjKvqgPQuLhnnX1y79J=qOT8szPvkHY86VQ@xxxxxxxxxxxxxx/

but did not get an answer.

>      You can check this using: https://github.com/ecree-solarflare/xmastree

If we have to check for that, then please have this patch revived and merged:

  https://lore.kernel.org/lkml/1478242438.1924.31.camel@xxxxxxxxxxx/

Personally, I am not willing to apply an out of tree linter for one
single use case.

>      In this case I think it would be:
>
>         struct virtio_can_priv *priv = netdev_priv(ndev);
>         struct device *dev = &priv->vdev->dev;
>         struct scatterlist sg_out[1];
>         struct scatterlist sg_in[1];
>         struct scatterlist *sgs[2];
>         struct virtqueue *vq;
>         unsigned int len;
>         int err;
>
>         vq = priv->vqs[VIRTIO_CAN_QUEUE_CONTROL];