Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20211009131304.19729-1-mailhol.vincent@wanadoo.fr>
 <20211009131304.19729-2-mailhol.vincent@wanadoo.fr> <20211024183007.u5pvfnlawhf36lfn@pengutronix.de>
In-Reply-To: <20211024183007.u5pvfnlawhf36lfn@pengutronix.de>
From:   Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Date:   Tue, 26 Oct 2021 02:22:36 +0900
Message-ID: <CAMZ6RqLw+B8ZioOyMFzha67Om3c8eKEK4P53U9xHiVxB4NBhkA@mail.gmail.com>
Subject: Re: [PATCH v2 1/3] can: dev: replace can_priv::ctrlmode_static by can_get_static_ctrlmode()
To:     Marc Kleine-Budde <mkl@pengutronix.de>
Cc:     linux-can <linux-can@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Marc,

Welcome back on the mailing list, hope you had some nice
holidays! And also thanks a lot for your support over the last
few months on my other series to introduce the TDC netlink
interface :)

Le lun. 25 oct. 2021 à 03:30, Marc Kleine-Budde <mkl@pengutronix.de> a écrit :
>
> On 09.10.2021 22:13:02, Vincent Mailhol wrote:
> > The statically enabled features of a CAN controller can be retrieved
> > using below formula:
> >
> > | u32 ctrlmode_static = priv->ctrlmode & ~priv->ctrlmode_supported;
> >
> > As such, there is no need to store this information. This patch remove
> > the field ctrlmode_static of struct can_priv and provides, in
> > replacement, the inline function can_get_static_ctrlmode() which
> > returns the same value.
> >
> > A condition sine qua non for this to work is that the controller
> > static modes should never be set in can_priv::ctrlmode_supported. This
> > is already the case for existing drivers, however, we added a warning
> > message in can_set_static_ctrlmode() to check that.
>
> Please make the can_set_static_ctrlmode to return an error in case of a
> problem. Adjust the drivers using the function is this patch, too.

I didn't do so initially because this is more a static
configuration issue that should only occur during
development. Nonetheless, what you suggest is really simple.

I will just split the patch in two: one of the setter and one for
the getter and address your comments.


Yours sincerely,
Vincent Mailhol