FDT support for RTL8366 on etherswitch framework
AbandonedPublic
Actions

Authored by yamori813_yahoo.co.jp on Oct 13 2017, 12:48 AM.

Details

Reviewers

gonzo
loos
sgalabov
mizhka
adrian

Summary

FDT away for RTL8366. Also fixed FDT support gpioiic.c.

Boot log is this.

https://gist.github.com/yamori813/bd6ccdbd73dd02eebfdd0bd6d92411d8

Test Plan

on MZK-W300NAG(RT2880 + RTL8366SR)

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 12194
Build 12492: arc lint + arc unit

Event Timeline

yamori813_yahoo.co.jp created this revision.Oct 13 2017, 12:48 AM

All is fine, one improvement is possible (gpio_pins for non-fdt case).

sys/dev/gpio/gpioiic.c
61–67 ↗	(On Diff #33929)	Is it possible to make support of SCL/SDA as "struct gpio_pin" for non-FDT case as well?

This revision is now accepted and ready to land.Oct 16 2017, 12:05 PM

as far as if_rt changes and dts additions go, I am ok with this.

I don't like all the duplicate code in gpioiic.c.

Can you explain which bugs are you trying to fix ?

gpioiic seems to works fine with FDT, but if you want to use the new API, maybe you should consider something like what was done with gpioled (and its FDT counterpart).

This revision now requires changes to proceed.Oct 18 2017, 5:58 PM

In D12649#263981, @loos wrote:

I don't like all the duplicate code in gpioiic.c.

Can you explain which bugs are you trying to fix ?

gpioiic seems to works fine with FDT, but if you want to use the new API, maybe you should consider something like what was done with gpioled (and its FDT counterpart).

I think IVAR can't use on FDT. Current gpioiic.c is use IVAR. Tell me dts file on work current gpioiic.c.

Current code is crash in gpioiic_attach() at devi->pins[] access in at FDT build.

Other FDT base gpio code use simplebus or ofwbus by DRIVER_MODULE. But gpioiic not use this.

Custom dts files shouldn't be put to sys/gnu/dts. This directory is periodically synced with upstream tree, so custom changes may be hard to maintain. The right way to new .dts is to put it to boot/fdt/dts/mips. All include operators should just work

review refict

In D12649#264042, @gonzo wrote:

Custom dts files shouldn't be put to sys/gnu/dts. This directory is periodically synced with upstream tree, so custom changes may be hard to maintain. The right way to new .dts is to put it to boot/fdt/dts/mips. All include operators should just work

my dts file move to sys/boot/fdt/dts/mips

gpioiic is sepalate like gpioled

loos Please show me complete dts file use gpioiic.c.

sys/dev/gpio/gpioiic.c
61–67 ↗	(On Diff #33929)	I seem other non-FDT gpio code use directly value. I think good that is not change this implement.

I will put separate this revision.

yamori813_yahoo.co.jp mentioned this in D22618: RT2880 and RT3883 gigabit ethernet interface support.Dec 1 2019, 6:34 AM

Revision Contents
Changeset List

Path

Size

sys/

boot/

fdt/

dts/

mips/

MZK-W300NAG.dts

104 lines

conf/

files

3 lines

dev/

etherswitch/

rtl8366/

rtl8366rb.c

27 lines

gpio/

gpioiic_fdt.c

243 lines

rt/

if_rt.c

22 lines

Diff 34271

View Options

sys/boot/fdt/dts/mips/MZK-W300NAG.dts

Show All 26 Lines
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT		* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS		* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.		* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/		*/
#include "gve.h"		#include "gve.h"
#include "gve_adminq.h"		#include "gve_adminq.h"
#include "gve_dqo.h"		#include "gve_dqo.h"

#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.2\n"		#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.3\n"
#define GVE_VERSION_MAJOR 1		#define GVE_VERSION_MAJOR 1
#define GVE_VERSION_MINOR 3		#define GVE_VERSION_MINOR 3
#define GVE_VERSION_SUB 2		#define GVE_VERSION_SUB 3

#define GVE_DEFAULT_RX_COPYBREAK 256		#define GVE_DEFAULT_RX_COPYBREAK 256

/* Devices supported by this driver. */		/* Devices supported by this driver. */
static struct gve_dev {		static struct gve_dev {
uint16_t vendor_id;		uint16_t vendor_id;
uint16_t device_id;		uint16_t device_id;
const char *name;		const char *name;
▲ Show 20 Lines • Show All 206 Lines • ▼ Show 20 Lines	gve_adjust_tx_queues(struct gve_priv *priv, uint16_t new_queue_cnt)
}		}
priv->tx_cfg.num_queues = new_queue_cnt;		priv->tx_cfg.num_queues = new_queue_cnt;

err = gve_up(priv);		err = gve_up(priv);
if (err != 0)		if (err != 0)
gve_schedule_reset(priv);		gve_schedule_reset(priv);

return (err);		return (err);
		}

		int
		gve_adjust_ring_sizes(struct gve_priv *priv, uint16_t new_desc_cnt, bool is_rx)
		{
		int err;
		uint16_t prev_desc_cnt;

		GVE_IFACE_LOCK_ASSERT(priv->gve_iface_lock);

		gve_down(priv);

		if (is_rx) {
		gve_free_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		prev_desc_cnt = priv->rx_desc_cnt;
		priv->rx_desc_cnt = new_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->rx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		}
		} else {
		gve_free_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		prev_desc_cnt = priv->tx_desc_cnt;
		priv->tx_desc_cnt = new_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->tx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		}
		}

		if (err != 0) {
		device_printf(priv->dev, "Failed to allocate rings! Cannot start device back up!");
		gve_down(priv);
		markjUnsubmitted Done Inline Actions Is this gve_down() call needed, given that we already bought the interface down? markj: Is this gve_down() call needed, given that we already bought the interface down?
		return (err);
		}

		err = gve_up(priv);
		if (err != 0) {
		gve_schedule_reset(priv);
		return (err);
		}

		return (0);
}		}

static int		static int
gve_set_mtu(if_t ifp, uint32_t new_mtu)		gve_set_mtu(if_t ifp, uint32_t new_mtu)
{		{
struct gve_priv *priv = if_getsoftc(ifp);		struct gve_priv *priv = if_getsoftc(ifp);
const uint32_t max_problem_range = 8227;		const uint32_t max_problem_range = 8227;
const uint32_t min_problem_range = 7822;		const uint32_t min_problem_range = 7822;
▲ Show 20 Lines • Show All 755 Lines • Show Last 20 Lines

View Options

sys/conf/files

Show All 26 Lines
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT		* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS		* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.		* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/		*/
#include "gve.h"		#include "gve.h"
#include "gve_adminq.h"		#include "gve_adminq.h"
#include "gve_dqo.h"		#include "gve_dqo.h"

#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.2\n"		#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.3\n"
#define GVE_VERSION_MAJOR 1		#define GVE_VERSION_MAJOR 1
#define GVE_VERSION_MINOR 3		#define GVE_VERSION_MINOR 3
#define GVE_VERSION_SUB 2		#define GVE_VERSION_SUB 3

#define GVE_DEFAULT_RX_COPYBREAK 256		#define GVE_DEFAULT_RX_COPYBREAK 256

/* Devices supported by this driver. */		/* Devices supported by this driver. */
static struct gve_dev {		static struct gve_dev {
uint16_t vendor_id;		uint16_t vendor_id;
uint16_t device_id;		uint16_t device_id;
const char *name;		const char *name;
▲ Show 20 Lines • Show All 206 Lines • ▼ Show 20 Lines	gve_adjust_tx_queues(struct gve_priv *priv, uint16_t new_queue_cnt)
}		}
priv->tx_cfg.num_queues = new_queue_cnt;		priv->tx_cfg.num_queues = new_queue_cnt;

err = gve_up(priv);		err = gve_up(priv);
if (err != 0)		if (err != 0)
gve_schedule_reset(priv);		gve_schedule_reset(priv);

return (err);		return (err);
		}

		int
		gve_adjust_ring_sizes(struct gve_priv *priv, uint16_t new_desc_cnt, bool is_rx)
		{
		int err;
		uint16_t prev_desc_cnt;

		GVE_IFACE_LOCK_ASSERT(priv->gve_iface_lock);

		gve_down(priv);

		if (is_rx) {
		gve_free_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		prev_desc_cnt = priv->rx_desc_cnt;
		priv->rx_desc_cnt = new_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->rx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		}
		} else {
		gve_free_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		prev_desc_cnt = priv->tx_desc_cnt;
		priv->tx_desc_cnt = new_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->tx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		}
		}

		if (err != 0) {
		device_printf(priv->dev, "Failed to allocate rings! Cannot start device back up!");
		gve_down(priv);
		markjUnsubmitted Done Inline Actions Is this gve_down() call needed, given that we already bought the interface down? markj: Is this gve_down() call needed, given that we already bought the interface down?
		return (err);
		}

		err = gve_up(priv);
		if (err != 0) {
		gve_schedule_reset(priv);
		return (err);
		}

		return (0);
}		}

static int		static int
gve_set_mtu(if_t ifp, uint32_t new_mtu)		gve_set_mtu(if_t ifp, uint32_t new_mtu)
{		{
struct gve_priv *priv = if_getsoftc(ifp);		struct gve_priv *priv = if_getsoftc(ifp);
const uint32_t max_problem_range = 8227;		const uint32_t max_problem_range = 8227;
const uint32_t min_problem_range = 7822;		const uint32_t min_problem_range = 7822;
▲ Show 20 Lines • Show All 755 Lines • Show Last 20 Lines

View Options

sys/dev/etherswitch/rtl8366/rtl8366rb.c

Show All 26 Lines
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT		* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS		* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.		* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/		*/
#include "gve.h"		#include "gve.h"
#include "gve_adminq.h"		#include "gve_adminq.h"
#include "gve_dqo.h"		#include "gve_dqo.h"

#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.2\n"		#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.3\n"
#define GVE_VERSION_MAJOR 1		#define GVE_VERSION_MAJOR 1
#define GVE_VERSION_MINOR 3		#define GVE_VERSION_MINOR 3
#define GVE_VERSION_SUB 2		#define GVE_VERSION_SUB 3

#define GVE_DEFAULT_RX_COPYBREAK 256		#define GVE_DEFAULT_RX_COPYBREAK 256

/* Devices supported by this driver. */		/* Devices supported by this driver. */
static struct gve_dev {		static struct gve_dev {
uint16_t vendor_id;		uint16_t vendor_id;
uint16_t device_id;		uint16_t device_id;
const char *name;		const char *name;
▲ Show 20 Lines • Show All 206 Lines • ▼ Show 20 Lines	gve_adjust_tx_queues(struct gve_priv *priv, uint16_t new_queue_cnt)
}		}
priv->tx_cfg.num_queues = new_queue_cnt;		priv->tx_cfg.num_queues = new_queue_cnt;

err = gve_up(priv);		err = gve_up(priv);
if (err != 0)		if (err != 0)
gve_schedule_reset(priv);		gve_schedule_reset(priv);

return (err);		return (err);
		}

		int
		gve_adjust_ring_sizes(struct gve_priv *priv, uint16_t new_desc_cnt, bool is_rx)
		{
		int err;
		uint16_t prev_desc_cnt;

		GVE_IFACE_LOCK_ASSERT(priv->gve_iface_lock);

		gve_down(priv);

		if (is_rx) {
		gve_free_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		prev_desc_cnt = priv->rx_desc_cnt;
		priv->rx_desc_cnt = new_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->rx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		}
		} else {
		gve_free_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		prev_desc_cnt = priv->tx_desc_cnt;
		priv->tx_desc_cnt = new_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->tx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		}
		}

		if (err != 0) {
		device_printf(priv->dev, "Failed to allocate rings! Cannot start device back up!");
		gve_down(priv);
		markjUnsubmitted Done Inline Actions Is this gve_down() call needed, given that we already bought the interface down? markj: Is this gve_down() call needed, given that we already bought the interface down?
		return (err);
		}

		err = gve_up(priv);
		if (err != 0) {
		gve_schedule_reset(priv);
		return (err);
		}

		return (0);
}		}

static int		static int
gve_set_mtu(if_t ifp, uint32_t new_mtu)		gve_set_mtu(if_t ifp, uint32_t new_mtu)
{		{
struct gve_priv *priv = if_getsoftc(ifp);		struct gve_priv *priv = if_getsoftc(ifp);
const uint32_t max_problem_range = 8227;		const uint32_t max_problem_range = 8227;
const uint32_t min_problem_range = 7822;		const uint32_t min_problem_range = 7822;
▲ Show 20 Lines • Show All 755 Lines • Show Last 20 Lines

View Options

sys/dev/gpio/gpioiic_fdt.c

Show All 26 Lines
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT		* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS		* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.		* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/		*/
#include "gve.h"		#include "gve.h"
#include "gve_adminq.h"		#include "gve_adminq.h"
#include "gve_dqo.h"		#include "gve_dqo.h"

#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.2\n"		#define GVE_DRIVER_VERSION "GVE-FBSD-1.3.3\n"
#define GVE_VERSION_MAJOR 1		#define GVE_VERSION_MAJOR 1
#define GVE_VERSION_MINOR 3		#define GVE_VERSION_MINOR 3
#define GVE_VERSION_SUB 2		#define GVE_VERSION_SUB 3

#define GVE_DEFAULT_RX_COPYBREAK 256		#define GVE_DEFAULT_RX_COPYBREAK 256

/* Devices supported by this driver. */		/* Devices supported by this driver. */
static struct gve_dev {		static struct gve_dev {
uint16_t vendor_id;		uint16_t vendor_id;
uint16_t device_id;		uint16_t device_id;
const char *name;		const char *name;
▲ Show 20 Lines • Show All 206 Lines • ▼ Show 20 Lines	gve_adjust_tx_queues(struct gve_priv *priv, uint16_t new_queue_cnt)
}		}
priv->tx_cfg.num_queues = new_queue_cnt;		priv->tx_cfg.num_queues = new_queue_cnt;

err = gve_up(priv);		err = gve_up(priv);
if (err != 0)		if (err != 0)
gve_schedule_reset(priv);		gve_schedule_reset(priv);

return (err);		return (err);
		}

		int
		gve_adjust_ring_sizes(struct gve_priv *priv, uint16_t new_desc_cnt, bool is_rx)
		{
		int err;
		uint16_t prev_desc_cnt;

		GVE_IFACE_LOCK_ASSERT(priv->gve_iface_lock);

		gve_down(priv);

		if (is_rx) {
		gve_free_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		prev_desc_cnt = priv->rx_desc_cnt;
		priv->rx_desc_cnt = new_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->rx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_rx_rings(priv, 0, priv->rx_cfg.num_queues);
		}
		} else {
		gve_free_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		prev_desc_cnt = priv->tx_desc_cnt;
		priv->tx_desc_cnt = new_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		if (err != 0) {
		device_printf(priv->dev,
		"Failed to allocate rings. Trying to start back up with previous ring size.");
		priv->tx_desc_cnt = prev_desc_cnt;
		err = gve_alloc_tx_rings(priv, 0, priv->tx_cfg.num_queues);
		}
		}

		if (err != 0) {
		device_printf(priv->dev, "Failed to allocate rings! Cannot start device back up!");
		gve_down(priv);
		markjUnsubmitted Done Inline Actions Is this gve_down() call needed, given that we already bought the interface down? markj: Is this gve_down() call needed, given that we already bought the interface down?
		return (err);
		}

		err = gve_up(priv);
		if (err != 0) {
		gve_schedule_reset(priv);
		return (err);
		}

		return (0);
}		}

static int		static int
gve_set_mtu(if_t ifp, uint32_t new_mtu)		gve_set_mtu(if_t ifp, uint32_t new_mtu)
{		{
struct gve_priv *priv = if_getsoftc(ifp);		struct gve_priv *priv = if_getsoftc(ifp);
const uint32_t max_problem_range = 8227;		const uint32_t max_problem_range = 8227;
const uint32_t min_problem_range = 7822;		const uint32_t min_problem_range = 7822;
▲ Show 20 Lines • Show All 755 Lines • Show Last 20 Lines

View Options

FDT support for RTL8366 on etherswitch frameworkAbandonedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 34271

sys/boot/fdt/dts/mips/MZK-W300NAG.dts

sys/conf/files

sys/dev/etherswitch/rtl8366/rtl8366rb.c

sys/dev/gpio/gpioiic_fdt.c

sys/dev/rt/if_rt.c

FDT support for RTL8366 on etherswitch framework
AbandonedPublic
Actions

Revision Contents
Changeset List