linux-kernel - Re: [PATCH v3, 2/2] net: Add dm9051 driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 12 Dec 2021 03:23:39 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     JosephCHANG <josright123@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Rob Herring <robh+dt@...nel.org>, joseph_chang@...icom.com.tw,
        netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3, 2/2] net: Add dm9051 driver

> +/* netdev_ops
> + */
> +static int dm9051_open(struct net_device *dev);
> +static int dm9051_stop(struct net_device *dev);
> +static netdev_tx_t dm9051_start_xmit(struct sk_buff *skb, struct net_device *dev);
> +static void dm9051_set_multicast_list_schedule(struct net_device *dev);
> +static int dm9051_set_mac_address(struct net_device *dev, void *p);

You should not need these. Move the code around so the functions come
before there first use.

> +/* carrier
> + */
> +#define	dm_carrier_init(db)			mii_check_link(&(db)->mii)
> +#define	dm_carrier_poll(db)			mii_check_link(&(db)->mii)

I requested you make use of phylib. Once you do, these will go away.

> +#define	dm_carrier_off(dev)			netif_carrier_off(dev)

No wrappers around standard functions. Also, once you use phylib, it
will take care of the carrier for you.

> +
> +/* xmit support
> + */
> +#define	dm_sk_buff_head_init(db)		skb_queue_head_init(&(db)->txq)
> +#define	dm_sk_buff_get(db)			skb_dequeue(&(db)->txq)
> +#define	dm_sk_buff_set(db, skb)			skb_queue_tail(&(db)->txq, skb)

These wrappers should also be removed.

> +/* spi transfers
> + */
> +#define ior					std_spi_read_reg			// read reg
> +#define iior					disp_spi_read_reg			// read disp
> +#define iow					std_spi_write_reg			// write reg
> +#define dm9inblk				std_read_rx_buf_ncpy			// read buff
> +#define dm9outblk				std_write_tx_buf			// write buf
> +
> +#define	ncr_reg_reset(db)			iow(db, DM9051_NCR, NCR_RST)		// reset
> +#define	mbd_reg_byte(db)			iow(db, DM9051_MBNDRY, MBNDRY_BYTE)	// MemBound
> +#define	fcr_reg_enable(db)			iow(db, DM9051_FCR, FCR_FLOW_ENABLE)	// FlowCtrl
> +#define	ppcr_reg_seeting(db)			iow(db, DM9051_PPCR, PPCR_PAUSE_COUNT)	// PauPktCn
> +#define	isr_reg_clear_to_stop_mrcmd(db)		iow(db, DM9051_ISR, 0xff)		// ClearISR
> +#define rcr_reg_stop(db)			iow(db, DM9051_RCR, RCR_RX_DISABLE)	// DisabRX
> +#define imr_reg_stop(db)			iow(db, DM9051_IMR, IMR_PAR)		// DisabAll
> +#define rcr_reg_start(db, rcr_all)		iow(db, DM9051_RCR, rcr_all)		// EnabRX
> +#define imr_reg_start(db, imr_all)		iow(db, DM9051_IMR, imr_all)		// Re-enab
> +#define	intcr_reg_setval(db)			iow(db, DM9051_INTCR, INTCR_POL_LOW)	// INTCR
> +#define	ledcr_reg_setting(db, lcr_all)		iow(db, DM9051_LMCR, lcr_all)		// LEDMode1

Please remove all these wrapper.

       Andrew