[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <dleftjd010vdpk.fsf%l.stelmach@samsung.com>
Date: Fri, 30 Oct 2020 00:01:43 +0100
From: Lukasz Stelmach <l.stelmach@...sung.com>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Andrew Lunn <andrew@...n.ch>, jim.cromie@...il.com,
Heiner Kallweit <hkallweit1@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Kukjin Kim <kgene@...nel.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
Russell King <linux@...linux.org.uk>, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org,
Marek Szyprowski <m.szyprowski@...sung.com>,
Bartłomiej Żolnierkiewicz
<b.zolnierkie@...sung.com>
Subject: Re: [PATCH v4 3/5] net: ax88796c: ASIX AX88796C SPI Ethernet
Adapter Driver
It was <2020-10-29 czw 18:27>, when Marc Kleine-Budde wrote:
> On 10/28/20 10:40 PM, Łukasz Stelmach wrote:
>> ASIX AX88796[1] is a versatile ethernet adapter chip, that can be
>> connected to a CPU with a 8/16-bit bus or with an SPI. This driver
>> supports SPI connection.
>>
>> The driver has been ported from the vendor kernel for ARTIK5[2]
>> boards. Several changes were made to adapt it to the current kernel
>> which include:
>>
>> + updated DT configuration,
>> + clock configuration moved to DT,
>> + new timer, ethtool and gpio APIs,
>> + dev_* instead of pr_* and custom printk() wrappers,
>> + removed awkward vendor power managemtn.
>>
>> [1]
>> https://protect2.fireeye.com/v1/url?k=9aaa9891-c7611faf-9aab13de-0cc47a31309a-7f8f6d6347765df4&q=1&e=78d1d40c-ff31-47e7-91fd-0c29963c1913&u=https%3A%2F%2Fwww.asix.com.tw%2Fproducts.php%3Fop%3DpItemdetail%26PItemID%3D104%3B65%3B86%26PLine%3D65
>> [2]
>> https://protect2.fireeye.com/v1/url?k=407e4fb6-1db5c888-407fc4f9-0cc47a31309a-aaf46a5c37be27ea&q=1&e=78d1d40c-ff31-47e7-91fd-0c29963c1913&u=https%3A%2F%2Fgit.tizen.org%2Fcgit%2Fprofile%2Fcommon%2Fplatform%2Fkernel%2Flinux-3.10-artik%2F
>>
>> The other ax88796 driver is for NE2000 compatible AX88796L chip. These
>> chips are not compatible. Hence, two separate drivers are required.
>>
>> Signed-off-by: Łukasz Stelmach <l.stelmach@...sung.com>
>> ---
>> MAINTAINERS | 6 +
>> drivers/net/ethernet/Kconfig | 1 +
>> drivers/net/ethernet/Makefile | 1 +
>> drivers/net/ethernet/asix/Kconfig | 22 +
>> drivers/net/ethernet/asix/Makefile | 6 +
>> drivers/net/ethernet/asix/ax88796c_ioctl.c | 197 ++++
>> drivers/net/ethernet/asix/ax88796c_ioctl.h | 26 +
>> drivers/net/ethernet/asix/ax88796c_main.c | 1144 ++++++++++++++++++++
>> drivers/net/ethernet/asix/ax88796c_main.h | 578 ++++++++++
>> drivers/net/ethernet/asix/ax88796c_spi.c | 111 ++
>> drivers/net/ethernet/asix/ax88796c_spi.h | 69 ++
>> 11 files changed, 2161 insertions(+)
>> create mode 100644 drivers/net/ethernet/asix/Kconfig
>> create mode 100644 drivers/net/ethernet/asix/Makefile
>> create mode 100644 drivers/net/ethernet/asix/ax88796c_ioctl.c
>> create mode 100644 drivers/net/ethernet/asix/ax88796c_ioctl.h
>> create mode 100644 drivers/net/ethernet/asix/ax88796c_main.c
>> create mode 100644 drivers/net/ethernet/asix/ax88796c_main.h
>> create mode 100644 drivers/net/ethernet/asix/ax88796c_spi.c
>> create mode 100644 drivers/net/ethernet/asix/ax88796c_spi.h
>
> [...]
>
>> +enum watchdog_state {
>> + chk_link = 0,
>> + chk_cable,
>> + ax_nop,
>> +};
>> +
>> +struct ax88796c_device {
>> + struct resource *addr_res; /* resources found */
>> + struct resource *addr_req; /* resources requested */
>> + struct resource *irq_res;
>> +
>> + struct spi_device *spi;
>> + struct net_device *ndev;
>> + struct net_device_stats stats;
>> +
>> + struct timer_list watchdog;
>> + enum watchdog_state w_state;
>> + size_t w_ticks;
>
> are these used?
>
Nope. Removed. Thanks.
>> +
>> + struct work_struct ax_work;
>> +
>> + struct mutex spi_lock; /* device access */
>> +
>> + struct sk_buff_head tx_wait_q;
>> +
>> + struct axspi_data ax_spi;
>> +
>> + struct mii_bus *mdiobus;
>> + struct phy_device *phydev;
>> +
>> + int msg_enable;
>> +
>> + u16 seq_num;
>> +
>> + u8 multi_filter[AX_MCAST_FILTER_SIZE];
>> +
>> + int link;
>> + int speed;
>> + int duplex;
>> + int pause;
>> + int asym_pause;
>> + int flowctrl;
>> + #define AX_FC_NONE 0
>> + #define AX_FC_RX BIT(0)
>> + #define AX_FC_TX BIT(1)
>> + #define AX_FC_ANEG BIT(2)
>> +
>> + unsigned long capabilities;
>> + #define AX_CAP_DMA BIT(0)
>> + #define AX_CAP_COMP BIT(1)
>> + #define AX_CAP_BIDIR BIT(2)
>> +
>> + u8 plat_endian;
>> + #define PLAT_LITTLE_ENDIAN 0
>> + #define PLAT_BIG_ENDIAN 1
>> +
>> + unsigned long flags;
>> + #define EVENT_INTR BIT(0)
>> + #define EVENT_TX BIT(1)
>> + #define EVENT_SET_MULTI BIT(2)
>> +
>> +};
>> +
>> +#define to_ax88796c_device(ndev) ((struct ax88796c_device *)netdev_priv(ndev))
>> +
>> +enum skb_state {
>> + illegal = 0,
>> + tx_done,
>> + rx_done,
>> + rx_err,
>> +};
>> +
>> +struct skb_data {
>> + enum skb_state state;
>> + struct net_device *ndev;
>> + struct sk_buff *skb;
>> + size_t len;
>> + dma_addr_t phy_addr;
>
> unused?
>
> [...]
>
Ditto.
>> diff --git a/drivers/net/ethernet/asix/ax88796c_spi.c b/drivers/net/ethernet/asix/ax88796c_spi.c
>> new file mode 100644
>> index 000000000000..1a20bbeb4dc1
>> --- /dev/null
>> +++ b/drivers/net/ethernet/asix/ax88796c_spi.c
>> @@ -0,0 +1,111 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2010 ASIX Electronics Corporation
>> + * Copyright (c) 2020 Samsung Electronics Co., Ltd.
>> + *
>> + * ASIX AX88796C SPI Fast Ethernet Linux driver
>> + */
>> +
>> +#define pr_fmt(fmt) "ax88796c: " fmt
>> +
>> +#include <linux/string.h>
>> +#include <linux/spi/spi.h>
>> +
>> +#include "ax88796c_spi.h"
>> +
>> +/* driver bus management functions */
>> +int axspi_wakeup(const struct axspi_data *ax_spi)
>> +{
>> + u8 tx_buf;
>> + int ret;
>> +
>> + tx_buf = AX_SPICMD_EXIT_PWD; /* OP */
>> + ret = spi_write(ax_spi->spi, &tx_buf, 1);
>
> spi_write() needs a DMA safe buffer.
>
Done.
>> + if (ret)
>> + dev_err(&ax_spi->spi->dev, "%s() failed: ret = %d\n", __func__, ret);
>> + return ret;
>> +}
>> +
>> +int axspi_read_status(const struct axspi_data *ax_spi, struct spi_status *status)
>> +{
>> + u8 tx_buf;
>> + int ret;
>> +
>> + /* OP */
>> + tx_buf = AX_SPICMD_READ_STATUS;
>> + ret = spi_write_then_read(ax_spi->spi, &tx_buf, 1, (u8 *)&status, 3);
>> + if (ret)
>> + dev_err(&ax_spi->spi->dev, "%s() failed: ret = %d\n", __func__, ret);
>> + else
>> + le16_to_cpus(&status->isr);
>> +
>> + return ret;
>> +}
>> +
>> +int axspi_read_rxq(struct axspi_data *ax_spi, void *data, int len)
>> +{
>> + struct spi_transfer *xfer = ax_spi->spi_rx_xfer;
>> + int ret;
>> +
>> + memcpy(ax_spi->cmd_buf, rx_cmd_buf, 5);
>> +
>> + xfer->tx_buf = ax_spi->cmd_buf;
>> + xfer->rx_buf = NULL;
>> + xfer->len = ax_spi->comp ? 2 : 5;
>> + xfer->bits_per_word = 8;
>> + spi_message_add_tail(xfer, &ax_spi->rx_msg);
>> +
>> + xfer++;
>> + xfer->rx_buf = data;
>> + xfer->tx_buf = NULL;
>> + xfer->len = len;
>> + xfer->bits_per_word = 8;
>> + spi_message_add_tail(xfer, &ax_spi->rx_msg);
>> + ret = spi_sync(ax_spi->spi, &ax_spi->rx_msg);
>> + if (ret)
>> + dev_err(&ax_spi->spi->dev, "%s() failed: ret = %d\n", __func__, ret);
>> +
>> + return ret;
>> +}
>> +
>> +int axspi_write_txq(const struct axspi_data *ax_spi, void *data, int len)
>> +{
>> + return spi_write(ax_spi->spi, data, len);
>> +}
>> +
>> +u16 axspi_read_reg(const struct axspi_data *ax_spi, u8 reg)
>> +{
>> + u8 tx_buf[4];
>> + u16 rx_buf = 0;
>> + int ret;
>> + int len = ax_spi->comp ? 3 : 4;
>> +
>> + tx_buf[0] = 0x03; /* OP code read register */
>> + tx_buf[1] = reg; /* register address */
>> + tx_buf[2] = 0xFF; /* dumy cycle */
>> + tx_buf[3] = 0xFF; /* dumy cycle */
>> + ret = spi_write_then_read(ax_spi->spi, tx_buf, len, (u8 *)&rx_buf, 2);
>> + if (ret)
>> + dev_err(&ax_spi->spi->dev, "%s() failed: ret = %d\n", __func__, ret);
>> + else
>> + le16_to_cpus(&rx_buf);
>> +
>> + return rx_buf;
>> +}
>> +
>> +int axspi_write_reg(const struct axspi_data *ax_spi, u8 reg, u16 value)
>> +{
>> + u8 tx_buf[4];
>> + int ret;
>> +
>> + tx_buf[0] = AX_SPICMD_WRITE_REG; /* OP code read register */
>> + tx_buf[1] = reg; /* register address */
>> + tx_buf[2] = value;
>> + tx_buf[3] = value >> 8;
>> +
>> + ret = spi_write(ax_spi->spi, tx_buf, 4);
>
> I think you need DMA safe mem for spi_write().
"Moved" the bufferers to axspi_data struct.
Thank you.
--
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
Download attachment "signature.asc" of type "application/pgp-signature" (488 bytes)
Powered by blists - more mailing lists