lkml.org 
[lkml]   [2023]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH V5] can: usb: f81604: add Fintek F81604 support
From
Hi Vincent, Michal and Marc,

Vincent MAILHOL 於 2023/4/21 下午 03:30 寫道:
> Hi Peter and Michal,
>
> On Fry. 21 Apr. 2023 at 12:14, Peter Hong <peter_hong@fintek.com.tw> wrote:
>> Hi Vincent,
>>
>> Vincent MAILHOL 於 2023/4/20 下午 08:02 寫道:
>>> Hi Peter,
>>>
>>> Here are my comments. Now, it is mostly nitpicks. I guess that this is
>>> the final round.
>>>
>>> On Thu. 20 avr. 2023 at 11:44, Ji-Ze Hong (Peter Hong)
>>> <peter_hong@fintek.com.tw> wrote:
>>>> +static void f81604_handle_tx(struct f81604_port_priv *priv,
>>>> + struct f81604_int_data *data)
>>>> +{
>>>> + struct net_device *netdev = priv->netdev;
>>>> + struct net_device_stats *stats;
>>>> +
>>>> + stats = &netdev->stats;
>>> Merge the declaration with the initialization.
>> If I merge initialization into declaration, it's may violation RCT?
>> How could I change about this ?
> @Michal: You requested RTC in:
>
> https://lore.kernel.org/linux-can/ZBgKSqaFiImtTThv@localhost.localdomain/
>
> I looked at the kernel documentation but I could not find "Reverse
> Chistmas Tree". Can you point me to where this is defined?
>
> In the above case, I do not think RCT should apply.
>
> I think that this:
>
> struct net_device *netdev = priv->netdev;
> struct net_device_stats *stats = &netdev->stats;
>
> Is better than that:
>
> struct net_device *netdev = priv->netdev;
> struct net_device_stats *stats;
>
> stats = &netdev->stats;
>
> Arbitrarily splitting the definition and assignment does not make sense to me.
>
> Thank you for your comments.

The RCT coding style seems a bit confuse. How about refactoring of next
step? @Marc ?


Thanks,

\
 
 \ /
  Last update: 2023-05-02 04:55    [W:0.047 / U:1.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site