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: <20211003044049.568441-1-mailhol.vincent@wanadoo.fr>
In-Reply-To: <20211003044049.568441-1-mailhol.vincent@wanadoo.fr>
From:   Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Date:   Sat, 9 Oct 2021 14:37:06 +0900
Message-ID: <CAMZ6RqKm+nLPd2oHgNebeDh2hSOMVnV7cJn12FM6NLpVaOz2iA@mail.gmail.com>
Subject: Re: [PATCH v1] can: netlink: report the CAN controller mode supported flags
To:     Marc Kleine-Budde <mkl@pengutronix.de>,
        linux-can <linux-can@vger.kernel.org>
Cc:     netdev <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun. 3 Oct 2021 at 13:40, Vincent Mailhol <mailhol.vincent@wanadoo.fr> wrote:
> This patch introduces a method for the user to check both the
> supported and the static capabilities.
>
> Currently, the CAN netlink interface provides no easy ways to check
> the capabilities of a given controller. The only method from the
> command line is to try each CAN_CTRLMODE_ individually to check
> whether the netlink interface returns an -EOPNOTSUPP error or not
> (alternatively, one may find it easier to directly check the source
> code of the driver instead...)
>
> It appears that, currently, the struct can_ctrlmode::mask field is
> only used in one direction: from the userland to the kernel. So we can
> just reuse this field in the other direction (from the kernel to
> userland). But, because the semantic is different, we use a union to
> give this field a proper name: supported.
>
> Below table explains how the two fields can_ctrlmode::supported and
> can_ctrlmode::flags, when masked with any of the CAN_CTRLMODE_* bit
> flags, allow us to identify both the supported and the static
> capabilities:
>
>  supported &    flags &         Controller capabilities
>  CAN_CTRLMODE_* CAN_CTRLMODE_*
>  ------------------------------------------------------------------------
>  false          false           Feature not supported (always disabled)
>  false          true            Static feature (always enabled)
>  true           false           Feature supported but disabled
>  true           true            Feature supported and enabled
>
> N.B.: This patch relies on the fact that a given CAN_CTRLMODE_*
> feature can not be set for both can_priv::ctrlmode_supported and
> can_priv::ctrlmode_static at the same time. c.f. comments in struct
> can_priv [1]. Else, there would be no way to distinguish which
> features were statically enabled.

Actually, can_priv::ctrlmode_static can be derived from the other
ctrlmode fields. I will send a v2 in which I will add a patch to
replace that field with an inline function.

Yours sincerely,
Vincent Mailhol