パッチ投げたら指導コメント入った
しかも速攻でした。
Hello. Just some trivial notes:
ついでとは言え、色々指導が入るのは色々な意味で有り難いです。
一つめは出力制御文字列に関するコメント。%lx じゃなくて %p 使えば的ソレ。
> diff --git a/drivers/staging/et131x/et131x.c b/drivers/staging/et131x/et131x.c [] > @@ -2553,8 +2555,8 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) [] > - printk(KERN_INFO "Packet Status Ring %lx\n", > - (unsigned long) rx_ring->ps_ring_physaddr); > + pr_info("Packet Status Ring %lx\n", > + (unsigned long) rx_ring->ps_ring_physaddr); > @@ -2574,7 +2576,7 @@ static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter) [] > - printk(KERN_INFO "PRS %lx\n", (unsigned long)rx_ring->rx_status_bus); > + pr_info("PRS %lx\n", (unsigned long)rx_ring->rx_status_bus); These might as well use %p without a cast to unsigned long pr_info("Packet Status Ring: %p\n", rx_ring->ps_ring_phydaddr); [] pr_info("PRS: %p\n", rx_ring->rx_status_bus);
%p 手持ちの ANSI C 言語辞典で見てみたら_処理系依存_みたいな事が書いてあったのですが、最近ではポインタをナニするのであれば逆にこっち使え的風潮なのかどうなのか。
で、もう一つは struct net_device * を云々するのなら netdev_
[] > @@ -5186,8 +5188,8 @@ static int et131x_set_mac_addr(struct net_device *netdev, void *new_mac) [] > - printk(KERN_INFO "%s: Setting MAC address to %pM\n", > - netdev->name, netdev->dev_addr); > + pr_info("%s: Setting MAC address to %pM\n", > + netdev->name, netdev->dev_addr); Please use netdev_<level> when there is a struct net_device * available. netdev_info(netdev, "Setting MAC address to %pM\n", netdev->dev_addr);
プロトタイプは include/linux/netdevice.h で定義されてて、実体は net/core/dev.c で云々されている模様。define_netdev_printk_level マクロを使って
define_netdev_printk_level(netdev_info, KERN_INFO);
手続きの実体が定義される模様。
ということで
これから修正盛り込んでパッチをナニ。つうかこの程度ならテストは不要なのだろうか。
むむ
とりあえず色々な意味でルール的なナニを理解できていないので_checkpatch_な warning を、という部分に集中した方が良さげな気がしてきました。
dma_addr_t なナニを printk するのも lx じゃなくて llx というのが多い。でも Google 先生古いポインタしか教えてくれてなさげで微妙。
とりあえず
I will fix netdev_info only, because I don't know if I should use "%p".
みたいなリプ戻してパッチ作成の方向。