Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1697652

Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate

Show all headers | View raw

Hi Kurt,
On 07/26/2017 03:04 PM, Kurt Van Dijck wrote:
> 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.

Its "possible" that this subnode may have additional properties beyond
bitrates in the future. But your right as of now it is specifically
addressing max bit rates. The naming of this function and subnode is
based on "fixed-link". So "fixed" is just implying that certain
properties can't be changed.

> 
> 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.

I address this partially in my rev 2 that I've already sent. I'm now
using of_can_transceiver_fixed. The fact its of_ already implies it is
loading properties from device tree. So I don't think
of_can_load_transceiver really makes things clearer.

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

You mind throwing your two cents in the thread for my latest patch?
Specifically the conversation regarding naming the properties. A couple
of people prefer to not use "arbitration" in one of the property names.
Currently I believe there are two options on property names that can be
used and I'm open to a majority vote on which one to go with.

> 
> Kind regards,
> Kurt Van Dijck
> 

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/4] can: Add new binding to limit bit rate used Franklin S Cooper Jr <fcooper@ti.com> - 2017-07-20 01:40 +0200
  [PATCH 3/4] can: m_can: Update documentation to mention new fixed transceiver binding Franklin S Cooper Jr <fcooper@ti.com> - 2017-07-20 01:40 +0200
    Re: [PATCH 3/4] can: m_can: Update documentation to mention new  fixed transceiver binding Rob Herring <robh@kernel.org> - 2017-07-24 22:00 +0200
  [PATCH 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr <fcooper@ti.com> - 2017-07-20 01:40 +0200
    Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate Oliver Hartkopp <socketcan@hartkopp.net> - 2017-07-20 09:50 +0200
      Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr <fcooper@ti.com> - 2017-07-20 18:00 +0200
        Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be> - 2017-07-26 22:30 +0200
          Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr <fcooper@ti.com> - 2017-07-27 03:50 +0200
    Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-07-20 12:00 +0200
      Re: [PATCH 1/4] can: dev: Add support for limiting configured bitrate Franklin S Cooper Jr <fcooper@ti.com> - 2017-07-20 18:10 +0200
  [PATCH 4/4] can: m_can: Add call to of_transceiver_is_fixed Franklin S Cooper Jr <fcooper@ti.com> - 2017-07-20 01:40 +0200

csiph-web