Date: Wed, 26 Jul 2017 22:04:03 +0200
From: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
To: Franklin S Cooper Jr <fcooper@ti.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, netdev@vger.kernel.org,
        linux-can@vger.kernel.org, wg@grandegger.com, mkl@pengutronix.de,
        robh+dt@kernel.org, quentin.schulz@free-electrons.com, andrew@lunn.ch
Subject: Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate
Message-ID: <20170726200403.GD6873@airbook.vandijck-laurijssen.be>
Mail-Followup-To: Franklin S Cooper Jr <fcooper@ti.com>,
        Oliver Hartkopp <socketcan@hartkopp.net>,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        netdev@vger.kernel.org, linux-can@vger.kernel.org,
        wg@grandegger.com, mkl@pengutronix.de, robh+dt@kernel.org,
        quentin.schulz@free-electrons.com, andrew@lunn.ch
References: <20170719233654.25908-1-fcooper@ti.com>
 <20170719233654.25908-2-fcooper@ti.com>
 <f45ea1b4-0f46-0002-a89b-b997f2693bb5@hartkopp.net>
 <055c00f5-0775-4e35-309c-0d1c8aa0afdf@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <055c00f5-0775-4e35-309c-0d1c8aa0afdf@ti.com>
User-Agent: Mutt/1.5.22 (2013-10-16)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 907
Lines: 37

Hi,

I know my response is late ...

> Hi Oliver
> On 07/20/2017 02:43 AM, Oliver Hartkopp wrote:
> > Hi Franklin,
> > 
> > On 07/20/2017 01:36 AM, Franklin S Cooper Jr wrote:
> > 
> >> +#ifdef CONFIG_OF
> >> +void of_transceiver_is_fixed(struct net_device *dev)
> >> +{
> > 
> > (..)
> > 
> >> +}
> >> +EXPORT_SYMBOL(of_transceiver_is_fixed);
> >> +#endif
> > 
> > I'm not sure about the naming here.
> > 
> > As this is a CAN transceiver related option it should be named accordingly:

I contest the the name too:
1) the can transceiver isn't fixed at all, it limited to the higher
bitrates.

2) of_can_transceiver_is_fixed suggests to test if a transceiver is
fixed, it does not suggest to load some properties from the device tree.
of_can_load_transceiver looks way more clear to me.

That's my opinion.
The important things, like the contents of the functions, look good.

Kind regards,
Kurt Van Dijck