From: Andrew Lunn <andrew@lunn.ch>
To: "Łukasz Stelmach" <l.stelmach@samsung.com>
Cc: jim.cromie@gmail.com, "Heiner Kallweit" <hkallweit1@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
"Jakub Kicinski" <kuba@kernel.org>,
"Rob Herring" <robh+dt@kernel.org>,
"Kukjin Kim" <kgene@kernel.org>,
"Krzysztof Kozlowski" <krzk@kernel.org>,
"Russell King" <linux@armlinux.org.uk>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-samsung-soc@vger.kernel.org,
"Bartłomiej Żolnierkiewicz" <b.zolnierkie@samsung.com>,
"Marek Szyprowski" <m.szyprowski@samsung.com>
Subject: Re: [PATCH v4 3/5] net: ax88796c: ASIX AX88796C SPI Ethernet Adapter Driver
Date: Thu, 29 Oct 2020 01:31:31 +0100 [thread overview]
Message-ID: <20201029003131.GF933237@lunn.ch> (raw)
In-Reply-To: <20201028214012.9712-4-l.stelmach@samsung.com>
> +static void
> +ax88796c_get_regs(struct net_device *ndev, struct ethtool_regs *regs, void *_p)
> +{
> + struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
> + u16 *p = _p;
> + int offset, i;
You missed a reverse christmass tree fix here.
> +static int comp;
> +static int msg_enable = NETIF_MSG_PROBE |
> + NETIF_MSG_LINK |
> + /* NETIF_MSG_TIMER | */
> + /* NETIF_MSG_IFDOWN | */
> + /* NETIF_MSG_IFUP | */
> + NETIF_MSG_RX_ERR |
> + NETIF_MSG_TX_ERR |
> + /* NETIF_MSG_TX_QUEUED | */
> + /* NETIF_MSG_INTR | */
> + /* NETIF_MSG_TX_DONE | */
> + /* NETIF_MSG_RX_STATUS | */
> + /* NETIF_MSG_PKTDATA | */
> + /* NETIF_MSG_HW | */
> + /* NETIF_MSG_WOL | */
> + 0;
You should probably delete anything which is commented out.
> +
> +static char *no_regs_list = "80018001,e1918001,8001a001,fc0d0000";
> +unsigned long ax88796c_no_regs_mask[AX88796C_REGDUMP_LEN / (sizeof(unsigned long) * 8)];
> +
> +module_param(comp, int, 0444);
> +MODULE_PARM_DESC(comp, "0=Non-Compression Mode, 1=Compression Mode");
I think you need to find a different way to configure this. How much
does compression bring you anyway?
> +module_param(msg_enable, int, 0444);
> +MODULE_PARM_DESC(msg_enable, "Message mask (see linux/netdevice.h for bitmap)");
I know a lot of drivers have msg_enable, but DaveM is generally
against module parameters. So i would remove this.
> +static void ax88796c_set_hw_multicast(struct net_device *ndev)
> +{
> + struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
> + u16 rx_ctl = RXCR_AB;
> + int mc_count = netdev_mc_count(ndev);
reverse christmass tree.
> +static struct sk_buff *
> +ax88796c_tx_fixup(struct net_device *ndev, struct sk_buff_head *q)
> +{
> + struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
> + struct sk_buff *skb, *tx_skb;
> + struct tx_pkt_info *info;
> + struct skb_data *entry;
> + int headroom;
> + int tailroom;
> + u8 need_pages;
> + u16 tol_len, pkt_len;
> + u8 padlen, seq_num;
> + u8 spi_len = ax_local->ax_spi.comp ? 1 : 4;
reverse christmass tree.
> +static int ax88796c_receive(struct net_device *ndev)
> +{
> + struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
> + struct sk_buff *skb;
> + struct skb_data *entry;
> + u16 w_count, pkt_len;
> + u8 pkt_cnt;
Reverse christmass tree
> +
> +static int ax88796c_process_isr(struct ax88796c_device *ax_local)
> +{
> + u16 isr;
> + u8 done = 0;
> + struct net_device *ndev = ax_local->ndev;
...
> +static irqreturn_t ax88796c_interrupt(int irq, void *dev_instance)
> +{
> + struct net_device *ndev = dev_instance;
> + struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
...
> +static int
> +ax88796c_open(struct net_device *ndev)
> +{
> + struct ax88796c_device *ax_local = to_ax88796c_device(ndev);
> + int ret;
> + unsigned long irq_flag = IRQF_SHARED;
> + int fc = AX_FC_NONE;
...
> +static int ax88796c_probe(struct spi_device *spi)
> +{
> + struct net_device *ndev;
> + struct ax88796c_device *ax_local;
> + char phy_id[MII_BUS_ID_SIZE + 3];
> + int ret;
> + u16 temp;
...
The mdio/phy/ethtool code looks O.K. now. I've not really looked at
any of the frame transfer code, so i cannot comment on that.
Andrew
next prev parent reply other threads:[~2020-10-29 0:31 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20201028214017eucas1p21a93b489acce80ff8a2fd1adfc9c1649@eucas1p2.samsung.com>
2020-10-28 21:40 ` [PATCH v4 0/5] AX88796C SPI Ethernet Adapter Łukasz Stelmach
[not found] ` <CGME20201028214016eucas1p1257ca6d0eacfbb97a42d97a5e45e0370@eucas1p1.samsung.com>
2020-10-28 21:40 ` [PATCH v4 1/5] dt-bindings: vendor-prefixes: Add asix prefix Łukasz Stelmach
[not found] ` <CGME20201028214017eucas1p251d5bd9f5f9db68da4ccefe8ee5e7c13@eucas1p2.samsung.com>
2020-10-28 21:40 ` [PATCH v4 2/5] dt-bindings: net: Add bindings for AX88796C SPI Ethernet Adapter Łukasz Stelmach
2020-10-29 15:28 ` Rob Herring
2020-10-29 17:06 ` Marc Kleine-Budde
[not found] ` <CGME20201029200708eucas1p1f00cdaf2c217056427dcd08f9d0d8bc9@eucas1p1.samsung.com>
2020-10-29 20:06 ` Lukasz Stelmach
[not found] ` <CGME20201028214016eucas1p19d2049a4edb4461b2424358e206dc59c@eucas1p1.samsung.com>
2020-10-28 21:40 ` [PATCH v4 3/5] net: ax88796c: ASIX AX88796C SPI Ethernet Adapter Driver Łukasz Stelmach
2020-10-29 0:31 ` Andrew Lunn [this message]
[not found] ` <CGME20201029131011eucas1p1194c5614ca8f5d3835f888c8d1c09fa1@eucas1p1.samsung.com>
2020-10-29 13:09 ` Lukasz Stelmach
[not found] ` <CGME20201029203142eucas1p138b7a69cf72e5ad0b1ecd8134adcbccf@eucas1p1.samsung.com>
2020-10-29 20:31 ` Lukasz Stelmach
2020-10-29 21:06 ` Andrew Lunn
2020-10-29 17:27 ` Marc Kleine-Budde
[not found] ` <CGME20201029230203eucas1p1d496586b195f2c01f1e5f69739c5ddfe@eucas1p1.samsung.com>
2020-10-29 23:01 ` Lukasz Stelmach
[not found] ` <CGME20201028214017eucas1p193f14480d56dfc49f07f27e4e7933ca5@eucas1p1.samsung.com>
2020-10-28 21:40 ` [PATCH v4 4/5] ARM: dts: exynos: Add Ethernet to Artik 5 board Łukasz Stelmach
[not found] ` <CGME20201028214017eucas1p16bc64d4596386177f4060689a6443098@eucas1p1.samsung.com>
2020-10-28 21:40 ` [PATCH v4 5/5] ARM: defconfig: Enable ax88796c driver Łukasz Stelmach
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201029003131.GF933237@lunn.ch \
--to=andrew@lunn.ch \
--cc=b.zolnierkie@samsung.com \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=hkallweit1@gmail.com \
--cc=jim.cromie@gmail.com \
--cc=kgene@kernel.org \
--cc=krzk@kernel.org \
--cc=kuba@kernel.org \
--cc=l.stelmach@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=m.szyprowski@samsung.com \
--cc=netdev@vger.kernel.org \
--cc=robh+dt@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).