Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230511151444.162882-1-Mikhail.Golubev-Ciuchea@opensynergy.com> <ZF0MXKkK1tEN6QyV@corigine.com>
In-Reply-To: <ZF0MXKkK1tEN6QyV@corigine.com>
From:   Vincent Mailhol <vincent.mailhol@gmail.com>
Date:   Fri, 12 May 2023 19:12:57 +0900
Message-ID: <CAMZ6Rq+6vv34Ps0G2SpB-9LHXYiD=esi604rm2tCE5Crp3QLvA@mail.gmail.com>
Subject: Re: [RFC PATCH v3] can: virtio: Initial virtio CAN driver.
To:     Simon Horman <simon.horman@corigine.com>
Cc:     Mikhail Golubev-Ciuchea <Mikhail.Golubev-Ciuchea@opensynergy.com>,
        virtio-dev@lists.oasis-open.org, linux-can@vger.kernel.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        virtualization@lists.linux-foundation.org,
        Wolfgang Grandegger <wg@grandegger.com>,
        Marc Kleine-Budde <mkl@pengutronix.de>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "Michael S . Tsirkin" <mst@redhat.com>,
        Jason Wang <jasowang@redhat.com>,
        Damir Shaikhutdinov <Damir.Shaikhutdinov@opensynergy.com>,
        Harald Mommer <harald.mommer@opensynergy.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Simon,

On Fri. 12 May 2023 at 00:45, Simon Horman <simon.horman@corigine.com> 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@mail.gmail.com/

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@perches.com/

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];