2018-07-14 11:45:55 +02:00
// SPDX-License-Identifier: GPL-2.0+
/* Realtek Simple Management Interface (SMI) driver
* It can be discussed how " simple " this interface is .
*
* The SMI protocol piggy - backs the MDIO MDC and MDIO signals levels
* but the protocol is not MDIO at all . Instead it is a Realtek
* pecularity that need to bit - bang the lines in a special way to
* communicate with the switch .
*
* ASICs we intend to support with this driver :
*
* RTL8366 - The original version , apparently
* RTL8369 - Similar enough to have the same datsheet as RTL8366
* RTL8366RB - Probably reads out " RTL8366 revision B " , has a quite
* different register layout from the other two
* RTL8366S - Is this " RTL8366 super " ?
* RTL8367 - Has an OpenWRT driver as well
* RTL8368S - Seems to be an alternative name for RTL8366RB
* RTL8370 - Also uses SMI
*
* Copyright ( C ) 2017 Linus Walleij < linus . walleij @ linaro . org >
* Copyright ( C ) 2010 Antti Seppälä < a . seppala @ gmail . com >
* Copyright ( C ) 2010 Roman Yeryomin < roman @ advem . lv >
* Copyright ( C ) 2011 Colin Leitner < colin . leitner @ googlemail . com >
* Copyright ( C ) 2009 - 2010 Gabor Juhos < juhosg @ openwrt . org >
*/
# include <linux/kernel.h>
# include <linux/module.h>
# include <linux/device.h>
# include <linux/spinlock.h>
# include <linux/skbuff.h>
# include <linux/of.h>
# include <linux/of_device.h>
# include <linux/of_mdio.h>
# include <linux/delay.h>
# include <linux/gpio/consumer.h>
# include <linux/platform_device.h>
# include <linux/regmap.h>
# include <linux/bitops.h>
# include <linux/if_bridge.h>
2022-01-28 03:04:59 -03:00
# include "realtek.h"
2018-07-14 11:45:55 +02:00
# define REALTEK_SMI_ACK_RETRY_COUNT 5
2022-01-28 03:04:59 -03:00
static inline void realtek_smi_clk_delay ( struct realtek_priv * priv )
2018-07-14 11:45:55 +02:00
{
2022-01-28 03:04:59 -03:00
ndelay ( priv - > clk_delay ) ;
2018-07-14 11:45:55 +02:00
}
2022-01-28 03:04:59 -03:00
static void realtek_smi_start ( struct realtek_priv * priv )
2018-07-14 11:45:55 +02:00
{
/* Set GPIO pins to output mode, with initial state:
* SCK = 0 , SDA = 1
*/
2022-01-28 03:04:59 -03:00
gpiod_direction_output ( priv - > mdc , 0 ) ;
gpiod_direction_output ( priv - > mdio , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
2018-07-14 11:45:55 +02:00
/* CLK 1: 0 -> 1, 1 -> 0 */
2022-01-28 03:04:59 -03:00
gpiod_set_value ( priv - > mdc , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 0 ) ;
realtek_smi_clk_delay ( priv ) ;
2018-07-14 11:45:55 +02:00
/* CLK 2: */
2022-01-28 03:04:59 -03:00
gpiod_set_value ( priv - > mdc , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdio , 0 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 0 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdio , 1 ) ;
2018-07-14 11:45:55 +02:00
}
2022-01-28 03:04:59 -03:00
static void realtek_smi_stop ( struct realtek_priv * priv )
2018-07-14 11:45:55 +02:00
{
2022-01-28 03:04:59 -03:00
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdio , 0 ) ;
gpiod_set_value ( priv - > mdc , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdio , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 0 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 1 ) ;
2018-07-14 11:45:55 +02:00
/* Add a click */
2022-01-28 03:04:59 -03:00
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 0 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 1 ) ;
2018-07-14 11:45:55 +02:00
/* Set GPIO pins to input mode */
2022-01-28 03:04:59 -03:00
gpiod_direction_input ( priv - > mdio ) ;
gpiod_direction_input ( priv - > mdc ) ;
2018-07-14 11:45:55 +02:00
}
2022-01-28 03:04:59 -03:00
static void realtek_smi_write_bits ( struct realtek_priv * priv , u32 data , u32 len )
2018-07-14 11:45:55 +02:00
{
for ( ; len > 0 ; len - - ) {
2022-01-28 03:04:59 -03:00
realtek_smi_clk_delay ( priv ) ;
2018-07-14 11:45:55 +02:00
/* Prepare data */
2022-01-28 03:04:59 -03:00
gpiod_set_value ( priv - > mdio , ! ! ( data & ( 1 < < ( len - 1 ) ) ) ) ;
realtek_smi_clk_delay ( priv ) ;
2018-07-14 11:45:55 +02:00
/* Clocking */
2022-01-28 03:04:59 -03:00
gpiod_set_value ( priv - > mdc , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
gpiod_set_value ( priv - > mdc , 0 ) ;
2018-07-14 11:45:55 +02:00
}
}
2022-01-28 03:04:59 -03:00
static void realtek_smi_read_bits ( struct realtek_priv * priv , u32 len , u32 * data )
2018-07-14 11:45:55 +02:00
{
2022-01-28 03:04:59 -03:00
gpiod_direction_input ( priv - > mdio ) ;
2018-07-14 11:45:55 +02:00
for ( * data = 0 ; len > 0 ; len - - ) {
u32 u ;
2022-01-28 03:04:59 -03:00
realtek_smi_clk_delay ( priv ) ;
2018-07-14 11:45:55 +02:00
/* Clocking */
2022-01-28 03:04:59 -03:00
gpiod_set_value ( priv - > mdc , 1 ) ;
realtek_smi_clk_delay ( priv ) ;
u = ! ! gpiod_get_value ( priv - > mdio ) ;
gpiod_set_value ( priv - > mdc , 0 ) ;
2018-07-14 11:45:55 +02:00
* data | = ( u < < ( len - 1 ) ) ;
}
2022-01-28 03:04:59 -03:00
gpiod_direction_output ( priv - > mdio , 0 ) ;
2018-07-14 11:45:55 +02:00
}
2022-01-28 03:04:59 -03:00
static int realtek_smi_wait_for_ack ( struct realtek_priv * priv )
2018-07-14 11:45:55 +02:00
{
int retry_cnt ;
retry_cnt = 0 ;
do {
u32 ack ;
2022-01-28 03:04:59 -03:00
realtek_smi_read_bits ( priv , 1 , & ack ) ;
2018-07-14 11:45:55 +02:00
if ( ack = = 0 )
break ;
if ( + + retry_cnt > REALTEK_SMI_ACK_RETRY_COUNT ) {
2022-01-28 03:04:59 -03:00
dev_err ( priv - > dev , " ACK timeout \n " ) ;
2018-07-14 11:45:55 +02:00
return - ETIMEDOUT ;
}
} while ( 1 ) ;
return 0 ;
}
2022-01-28 03:04:59 -03:00
static int realtek_smi_write_byte ( struct realtek_priv * priv , u8 data )
2018-07-14 11:45:55 +02:00
{
2022-01-28 03:04:59 -03:00
realtek_smi_write_bits ( priv , data , 8 ) ;
return realtek_smi_wait_for_ack ( priv ) ;
2018-07-14 11:45:55 +02:00
}
2022-01-28 03:04:59 -03:00
static int realtek_smi_write_byte_noack ( struct realtek_priv * priv , u8 data )
2018-07-14 11:45:55 +02:00
{
2022-01-28 03:04:59 -03:00
realtek_smi_write_bits ( priv , data , 8 ) ;
2018-07-14 11:45:55 +02:00
return 0 ;
}
2022-01-28 03:04:59 -03:00
static int realtek_smi_read_byte0 ( struct realtek_priv * priv , u8 * data )
2018-07-14 11:45:55 +02:00
{
u32 t ;
/* Read data */
2022-01-28 03:04:59 -03:00
realtek_smi_read_bits ( priv , 8 , & t ) ;
2018-07-14 11:45:55 +02:00
* data = ( t & 0xff ) ;
/* Send an ACK */
2022-01-28 03:04:59 -03:00
realtek_smi_write_bits ( priv , 0x00 , 1 ) ;
2018-07-14 11:45:55 +02:00
return 0 ;
}
2022-01-28 03:04:59 -03:00
static int realtek_smi_read_byte1 ( struct realtek_priv * priv , u8 * data )
2018-07-14 11:45:55 +02:00
{
u32 t ;
/* Read data */
2022-01-28 03:04:59 -03:00
realtek_smi_read_bits ( priv , 8 , & t ) ;
2018-07-14 11:45:55 +02:00
* data = ( t & 0xff ) ;
/* Send an ACK */
2022-01-28 03:04:59 -03:00
realtek_smi_write_bits ( priv , 0x01 , 1 ) ;
2018-07-14 11:45:55 +02:00
return 0 ;
}
2022-01-28 03:04:59 -03:00
static int realtek_smi_read_reg ( struct realtek_priv * priv , u32 addr , u32 * data )
2018-07-14 11:45:55 +02:00
{
unsigned long flags ;
u8 lo = 0 ;
u8 hi = 0 ;
int ret ;
2022-01-28 03:04:59 -03:00
spin_lock_irqsave ( & priv - > lock , flags ) ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
realtek_smi_start ( priv ) ;
2018-07-14 11:45:55 +02:00
/* Send READ command */
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , priv - > cmd_read ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
/* Set ADDR[7:0] */
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , addr & 0xff ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
/* Set ADDR[15:8] */
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , addr > > 8 ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
/* Read DATA[7:0] */
2022-01-28 03:04:59 -03:00
realtek_smi_read_byte0 ( priv , & lo ) ;
2018-07-14 11:45:55 +02:00
/* Read DATA[15:8] */
2022-01-28 03:04:59 -03:00
realtek_smi_read_byte1 ( priv , & hi ) ;
2018-07-14 11:45:55 +02:00
* data = ( ( u32 ) lo ) | ( ( ( u32 ) hi ) < < 8 ) ;
ret = 0 ;
out :
2022-01-28 03:04:59 -03:00
realtek_smi_stop ( priv ) ;
spin_unlock_irqrestore ( & priv - > lock , flags ) ;
2018-07-14 11:45:55 +02:00
return ret ;
}
2022-01-28 03:04:59 -03:00
static int realtek_smi_write_reg ( struct realtek_priv * priv ,
2018-07-14 11:45:55 +02:00
u32 addr , u32 data , bool ack )
{
unsigned long flags ;
int ret ;
2022-01-28 03:04:59 -03:00
spin_lock_irqsave ( & priv - > lock , flags ) ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
realtek_smi_start ( priv ) ;
2018-07-14 11:45:55 +02:00
/* Send WRITE command */
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , priv - > cmd_write ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
/* Set ADDR[7:0] */
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , addr & 0xff ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
/* Set ADDR[15:8] */
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , addr > > 8 ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
/* Write DATA[7:0] */
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , data & 0xff ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
/* Write DATA[15:8] */
if ( ack )
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte ( priv , data > > 8 ) ;
2018-07-14 11:45:55 +02:00
else
2022-01-28 03:04:59 -03:00
ret = realtek_smi_write_byte_noack ( priv , data > > 8 ) ;
2018-07-14 11:45:55 +02:00
if ( ret )
goto out ;
ret = 0 ;
out :
2022-01-28 03:04:59 -03:00
realtek_smi_stop ( priv ) ;
spin_unlock_irqrestore ( & priv - > lock , flags ) ;
2018-07-14 11:45:55 +02:00
return ret ;
}
/* There is one single case when we need to use this accessor and that
* is when issueing soft reset . Since the device reset as soon as we write
* that bit , no ACK will come back for natural reasons .
*/
2022-01-28 03:05:00 -03:00
static int realtek_smi_write_reg_noack ( void * ctx , u32 reg , u32 val )
2018-07-14 11:45:55 +02:00
{
2022-01-28 03:05:00 -03:00
return realtek_smi_write_reg ( ctx , reg , val , false ) ;
2018-07-14 11:45:55 +02:00
}
/* Regmap accessors */
static int realtek_smi_write ( void * ctx , u32 reg , u32 val )
{
2022-01-28 03:04:59 -03:00
struct realtek_priv * priv = ctx ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
return realtek_smi_write_reg ( priv , reg , val , true ) ;
2018-07-14 11:45:55 +02:00
}
static int realtek_smi_read ( void * ctx , u32 reg , u32 * val )
{
2022-01-28 03:04:59 -03:00
struct realtek_priv * priv = ctx ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
return realtek_smi_read_reg ( priv , reg , val ) ;
2018-07-14 11:45:55 +02:00
}
net: dsa: realtek: allow subdrivers to externally lock regmap
Currently there is no way for Realtek DSA subdrivers to serialize
consecutive regmap accesses. In preparation for a bugfix relating to
indirect PHY register access - which involves a series of regmap
reads and writes - add a facility for subdrivers to serialize their
regmap access.
Specifically, a mutex is added to the driver private data structure and
the standard regmap is initialized with custom lock/unlock ops which use
this mutex. Then, a "nolock" variant of the regmap is added, which is
functionally equivalent to the existing regmap except that regmap
locking is disabled. Functions that wish to serialize a sequence of
regmap accesses may then lock the newly introduced driver-owned mutex
before using the nolock regmap.
Doing things this way means that subdriver code that doesn't care about
serialized register access - i.e. the vast majority of code - needn't
worry about synchronizing register access with an external lock: it can
just continue to use the original regmap.
Another advantage of this design is that, while regmaps with locking
disabled do not expose a debugfs interface for obvious reasons, there
still exists the original regmap which does expose this interface. This
interface remains safe to use even combined with driver codepaths that
use the nolock regmap, because said codepaths will use the same mutex
to synchronize access.
With respect to disadvantages, it can be argued that having
near-duplicate regmaps is confusing. However, the naming is rather
explicit, and examples will abound.
Finally, while we are at it, rename realtek_smi_mdio_regmap_config to
realtek_smi_regmap_config. This makes it consistent with the naming
realtek_mdio_regmap_config in realtek-mdio.c.
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2022-02-21 19:46:30 +01:00
static void realtek_smi_lock ( void * ctx )
{
struct realtek_priv * priv = ctx ;
mutex_lock ( & priv - > map_lock ) ;
}
static void realtek_smi_unlock ( void * ctx )
{
struct realtek_priv * priv = ctx ;
mutex_unlock ( & priv - > map_lock ) ;
}
static const struct regmap_config realtek_smi_regmap_config = {
2018-07-14 11:45:55 +02:00
. reg_bits = 10 , /* A4..A0 R4..R0 */
. val_bits = 16 ,
. reg_stride = 1 ,
/* PHY regs are at 0x8000 */
. max_register = 0xffff ,
. reg_format_endian = REGMAP_ENDIAN_BIG ,
. reg_read = realtek_smi_read ,
. reg_write = realtek_smi_write ,
. cache_type = REGCACHE_NONE ,
net: dsa: realtek: allow subdrivers to externally lock regmap
Currently there is no way for Realtek DSA subdrivers to serialize
consecutive regmap accesses. In preparation for a bugfix relating to
indirect PHY register access - which involves a series of regmap
reads and writes - add a facility for subdrivers to serialize their
regmap access.
Specifically, a mutex is added to the driver private data structure and
the standard regmap is initialized with custom lock/unlock ops which use
this mutex. Then, a "nolock" variant of the regmap is added, which is
functionally equivalent to the existing regmap except that regmap
locking is disabled. Functions that wish to serialize a sequence of
regmap accesses may then lock the newly introduced driver-owned mutex
before using the nolock regmap.
Doing things this way means that subdriver code that doesn't care about
serialized register access - i.e. the vast majority of code - needn't
worry about synchronizing register access with an external lock: it can
just continue to use the original regmap.
Another advantage of this design is that, while regmaps with locking
disabled do not expose a debugfs interface for obvious reasons, there
still exists the original regmap which does expose this interface. This
interface remains safe to use even combined with driver codepaths that
use the nolock regmap, because said codepaths will use the same mutex
to synchronize access.
With respect to disadvantages, it can be argued that having
near-duplicate regmaps is confusing. However, the naming is rather
explicit, and examples will abound.
Finally, while we are at it, rename realtek_smi_mdio_regmap_config to
realtek_smi_regmap_config. This makes it consistent with the naming
realtek_mdio_regmap_config in realtek-mdio.c.
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2022-02-21 19:46:30 +01:00
. lock = realtek_smi_lock ,
. unlock = realtek_smi_unlock ,
} ;
static const struct regmap_config realtek_smi_nolock_regmap_config = {
. reg_bits = 10 , /* A4..A0 R4..R0 */
. val_bits = 16 ,
. reg_stride = 1 ,
/* PHY regs are at 0x8000 */
. max_register = 0xffff ,
. reg_format_endian = REGMAP_ENDIAN_BIG ,
. reg_read = realtek_smi_read ,
. reg_write = realtek_smi_write ,
. cache_type = REGCACHE_NONE ,
. disable_locking = true ,
2018-07-14 11:45:55 +02:00
} ;
static int realtek_smi_mdio_read ( struct mii_bus * bus , int addr , int regnum )
{
2022-01-28 03:04:59 -03:00
struct realtek_priv * priv = bus - > priv ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
return priv - > ops - > phy_read ( priv , addr , regnum ) ;
2018-07-14 11:45:55 +02:00
}
static int realtek_smi_mdio_write ( struct mii_bus * bus , int addr , int regnum ,
u16 val )
{
2022-01-28 03:04:59 -03:00
struct realtek_priv * priv = bus - > priv ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
return priv - > ops - > phy_write ( priv , addr , regnum , val ) ;
2018-07-14 11:45:55 +02:00
}
2022-01-28 03:05:00 -03:00
static int realtek_smi_setup_mdio ( struct dsa_switch * ds )
2018-07-14 11:45:55 +02:00
{
2022-01-28 03:05:00 -03:00
struct realtek_priv * priv = ds - > priv ;
2018-07-14 11:45:55 +02:00
struct device_node * mdio_np ;
int ret ;
2022-01-28 03:04:59 -03:00
mdio_np = of_get_compatible_child ( priv - > dev - > of_node , " realtek,smi-mdio " ) ;
2018-07-14 11:45:55 +02:00
if ( ! mdio_np ) {
2022-01-28 03:04:59 -03:00
dev_err ( priv - > dev , " no MDIO bus node \n " ) ;
2018-07-14 11:45:55 +02:00
return - ENODEV ;
}
2022-01-28 03:04:59 -03:00
priv - > slave_mii_bus = devm_mdiobus_alloc ( priv - > dev ) ;
if ( ! priv - > slave_mii_bus ) {
2019-01-16 11:27:08 +01:00
ret = - ENOMEM ;
goto err_put_node ;
}
2022-01-28 03:04:59 -03:00
priv - > slave_mii_bus - > priv = priv ;
priv - > slave_mii_bus - > name = " SMI slave MII " ;
priv - > slave_mii_bus - > read = realtek_smi_mdio_read ;
priv - > slave_mii_bus - > write = realtek_smi_mdio_write ;
snprintf ( priv - > slave_mii_bus - > id , MII_BUS_ID_SIZE , " SMI-%d " ,
2022-01-28 03:05:00 -03:00
ds - > index ) ;
2022-01-28 03:04:59 -03:00
priv - > slave_mii_bus - > dev . of_node = mdio_np ;
priv - > slave_mii_bus - > parent = priv - > dev ;
2022-01-28 03:05:00 -03:00
ds - > slave_mii_bus = priv - > slave_mii_bus ;
2022-01-28 03:04:59 -03:00
ret = devm_of_mdiobus_register ( priv - > dev , priv - > slave_mii_bus , mdio_np ) ;
2018-07-14 11:45:55 +02:00
if ( ret ) {
2022-01-28 03:04:59 -03:00
dev_err ( priv - > dev , " unable to register MDIO bus %s \n " ,
priv - > slave_mii_bus - > id ) ;
2019-01-16 11:27:08 +01:00
goto err_put_node ;
2018-07-14 11:45:55 +02:00
}
return 0 ;
2019-01-16 11:27:08 +01:00
err_put_node :
of_node_put ( mdio_np ) ;
return ret ;
2018-07-14 11:45:55 +02:00
}
static int realtek_smi_probe ( struct platform_device * pdev )
{
2022-01-28 03:04:59 -03:00
const struct realtek_variant * var ;
2018-07-14 11:45:55 +02:00
struct device * dev = & pdev - > dev ;
2022-01-28 03:04:59 -03:00
struct realtek_priv * priv ;
net: dsa: realtek: allow subdrivers to externally lock regmap
Currently there is no way for Realtek DSA subdrivers to serialize
consecutive regmap accesses. In preparation for a bugfix relating to
indirect PHY register access - which involves a series of regmap
reads and writes - add a facility for subdrivers to serialize their
regmap access.
Specifically, a mutex is added to the driver private data structure and
the standard regmap is initialized with custom lock/unlock ops which use
this mutex. Then, a "nolock" variant of the regmap is added, which is
functionally equivalent to the existing regmap except that regmap
locking is disabled. Functions that wish to serialize a sequence of
regmap accesses may then lock the newly introduced driver-owned mutex
before using the nolock regmap.
Doing things this way means that subdriver code that doesn't care about
serialized register access - i.e. the vast majority of code - needn't
worry about synchronizing register access with an external lock: it can
just continue to use the original regmap.
Another advantage of this design is that, while regmaps with locking
disabled do not expose a debugfs interface for obvious reasons, there
still exists the original regmap which does expose this interface. This
interface remains safe to use even combined with driver codepaths that
use the nolock regmap, because said codepaths will use the same mutex
to synchronize access.
With respect to disadvantages, it can be argued that having
near-duplicate regmaps is confusing. However, the naming is rather
explicit, and examples will abound.
Finally, while we are at it, rename realtek_smi_mdio_regmap_config to
realtek_smi_regmap_config. This makes it consistent with the naming
realtek_mdio_regmap_config in realtek-mdio.c.
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2022-02-21 19:46:30 +01:00
struct regmap_config rc ;
2018-07-14 11:45:55 +02:00
struct device_node * np ;
int ret ;
var = of_device_get_match_data ( dev ) ;
np = dev - > of_node ;
2022-01-28 03:04:59 -03:00
priv = devm_kzalloc ( dev , sizeof ( * priv ) + var - > chip_data_sz , GFP_KERNEL ) ;
if ( ! priv )
2018-07-14 11:45:55 +02:00
return - ENOMEM ;
2022-01-28 03:04:59 -03:00
priv - > chip_data = ( void * ) priv + sizeof ( * priv ) ;
net: dsa: realtek: allow subdrivers to externally lock regmap
Currently there is no way for Realtek DSA subdrivers to serialize
consecutive regmap accesses. In preparation for a bugfix relating to
indirect PHY register access - which involves a series of regmap
reads and writes - add a facility for subdrivers to serialize their
regmap access.
Specifically, a mutex is added to the driver private data structure and
the standard regmap is initialized with custom lock/unlock ops which use
this mutex. Then, a "nolock" variant of the regmap is added, which is
functionally equivalent to the existing regmap except that regmap
locking is disabled. Functions that wish to serialize a sequence of
regmap accesses may then lock the newly introduced driver-owned mutex
before using the nolock regmap.
Doing things this way means that subdriver code that doesn't care about
serialized register access - i.e. the vast majority of code - needn't
worry about synchronizing register access with an external lock: it can
just continue to use the original regmap.
Another advantage of this design is that, while regmaps with locking
disabled do not expose a debugfs interface for obvious reasons, there
still exists the original regmap which does expose this interface. This
interface remains safe to use even combined with driver codepaths that
use the nolock regmap, because said codepaths will use the same mutex
to synchronize access.
With respect to disadvantages, it can be argued that having
near-duplicate regmaps is confusing. However, the naming is rather
explicit, and examples will abound.
Finally, while we are at it, rename realtek_smi_mdio_regmap_config to
realtek_smi_regmap_config. This makes it consistent with the naming
realtek_mdio_regmap_config in realtek-mdio.c.
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2022-02-21 19:46:30 +01:00
mutex_init ( & priv - > map_lock ) ;
rc = realtek_smi_regmap_config ;
rc . lock_arg = priv ;
priv - > map = devm_regmap_init ( dev , NULL , priv , & rc ) ;
2022-01-28 03:04:59 -03:00
if ( IS_ERR ( priv - > map ) ) {
ret = PTR_ERR ( priv - > map ) ;
2018-07-14 11:45:55 +02:00
dev_err ( dev , " regmap init failed: %d \n " , ret ) ;
return ret ;
}
net: dsa: realtek: allow subdrivers to externally lock regmap
Currently there is no way for Realtek DSA subdrivers to serialize
consecutive regmap accesses. In preparation for a bugfix relating to
indirect PHY register access - which involves a series of regmap
reads and writes - add a facility for subdrivers to serialize their
regmap access.
Specifically, a mutex is added to the driver private data structure and
the standard regmap is initialized with custom lock/unlock ops which use
this mutex. Then, a "nolock" variant of the regmap is added, which is
functionally equivalent to the existing regmap except that regmap
locking is disabled. Functions that wish to serialize a sequence of
regmap accesses may then lock the newly introduced driver-owned mutex
before using the nolock regmap.
Doing things this way means that subdriver code that doesn't care about
serialized register access - i.e. the vast majority of code - needn't
worry about synchronizing register access with an external lock: it can
just continue to use the original regmap.
Another advantage of this design is that, while regmaps with locking
disabled do not expose a debugfs interface for obvious reasons, there
still exists the original regmap which does expose this interface. This
interface remains safe to use even combined with driver codepaths that
use the nolock regmap, because said codepaths will use the same mutex
to synchronize access.
With respect to disadvantages, it can be argued that having
near-duplicate regmaps is confusing. However, the naming is rather
explicit, and examples will abound.
Finally, while we are at it, rename realtek_smi_mdio_regmap_config to
realtek_smi_regmap_config. This makes it consistent with the naming
realtek_mdio_regmap_config in realtek-mdio.c.
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2022-02-21 19:46:30 +01:00
rc = realtek_smi_nolock_regmap_config ;
priv - > map_nolock = devm_regmap_init ( dev , NULL , priv , & rc ) ;
if ( IS_ERR ( priv - > map_nolock ) ) {
ret = PTR_ERR ( priv - > map_nolock ) ;
dev_err ( dev , " regmap init failed: %d \n " , ret ) ;
return ret ;
}
2018-07-14 11:45:55 +02:00
/* Link forward and backward */
2022-01-28 03:04:59 -03:00
priv - > dev = dev ;
priv - > clk_delay = var - > clk_delay ;
priv - > cmd_read = var - > cmd_read ;
priv - > cmd_write = var - > cmd_write ;
priv - > ops = var - > ops ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:05:00 -03:00
priv - > setup_interface = realtek_smi_setup_mdio ;
priv - > write_reg_noack = realtek_smi_write_reg_noack ;
2022-01-28 03:04:59 -03:00
dev_set_drvdata ( dev , priv ) ;
spin_lock_init ( & priv - > lock ) ;
2018-07-14 11:45:55 +02:00
/* TODO: if power is software controlled, set up any regulators here */
2022-02-13 23:20:11 -03:00
priv - > reset = devm_gpiod_get_optional ( dev , " reset " , GPIOD_OUT_LOW ) ;
2022-01-28 03:04:59 -03:00
if ( IS_ERR ( priv - > reset ) ) {
2018-07-14 11:45:55 +02:00
dev_err ( dev , " failed to get RESET GPIO \n " ) ;
2022-01-28 03:04:59 -03:00
return PTR_ERR ( priv - > reset ) ;
2018-07-14 11:45:55 +02:00
}
2022-02-13 23:20:11 -03:00
if ( priv - > reset ) {
gpiod_set_value ( priv - > reset , 1 ) ;
dev_dbg ( dev , " asserted RESET \n " ) ;
2022-02-13 23:20:12 -03:00
msleep ( REALTEK_HW_STOP_DELAY ) ;
2022-02-13 23:20:11 -03:00
gpiod_set_value ( priv - > reset , 0 ) ;
2022-02-13 23:20:12 -03:00
msleep ( REALTEK_HW_START_DELAY ) ;
2022-02-13 23:20:11 -03:00
dev_dbg ( dev , " deasserted RESET \n " ) ;
}
2018-07-14 11:45:55 +02:00
/* Fetch MDIO pins */
2022-01-28 03:04:59 -03:00
priv - > mdc = devm_gpiod_get_optional ( dev , " mdc " , GPIOD_OUT_LOW ) ;
if ( IS_ERR ( priv - > mdc ) )
return PTR_ERR ( priv - > mdc ) ;
priv - > mdio = devm_gpiod_get_optional ( dev , " mdio " , GPIOD_OUT_LOW ) ;
if ( IS_ERR ( priv - > mdio ) )
return PTR_ERR ( priv - > mdio ) ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
priv - > leds_disabled = of_property_read_bool ( np , " realtek,disable-leds " ) ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
ret = priv - > ops - > detect ( priv ) ;
2018-07-14 11:45:55 +02:00
if ( ret ) {
dev_err ( dev , " unable to detect switch \n " ) ;
return ret ;
}
2022-01-28 03:04:59 -03:00
priv - > ds = devm_kzalloc ( dev , sizeof ( * priv - > ds ) , GFP_KERNEL ) ;
if ( ! priv - > ds )
2018-07-14 11:45:55 +02:00
return - ENOMEM ;
2019-10-21 16:51:30 -04:00
2022-01-28 03:04:59 -03:00
priv - > ds - > dev = dev ;
priv - > ds - > num_ports = priv - > num_ports ;
priv - > ds - > priv = priv ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:05:02 -03:00
priv - > ds - > ops = var - > ds_ops_smi ;
2022-01-28 03:04:59 -03:00
ret = dsa_register_switch ( priv - > ds ) ;
2018-07-14 11:45:55 +02:00
if ( ret ) {
2021-11-29 11:30:17 +01:00
dev_err_probe ( dev , ret , " unable to register switch \n " ) ;
2018-07-14 11:45:55 +02:00
return ret ;
}
return 0 ;
}
static int realtek_smi_remove ( struct platform_device * pdev )
{
2022-01-28 03:04:59 -03:00
struct realtek_priv * priv = platform_get_drvdata ( pdev ) ;
net: dsa: be compatible with masters which unregister on shutdown
Lino reports that on his system with bcmgenet as DSA master and KSZ9897
as a switch, rebooting or shutting down never works properly.
What does the bcmgenet driver have special to trigger this, that other
DSA masters do not? It has an implementation of ->shutdown which simply
calls its ->remove implementation. Otherwise said, it unregisters its
network interface on shutdown.
This message can be seen in a loop, and it hangs the reboot process there:
unregister_netdevice: waiting for eth0 to become free. Usage count = 3
So why 3?
A usage count of 1 is normal for a registered network interface, and any
virtual interface which links itself as an upper of that will increment
it via dev_hold. In the case of DSA, this is the call path:
dsa_slave_create
-> netdev_upper_dev_link
-> __netdev_upper_dev_link
-> __netdev_adjacent_dev_insert
-> dev_hold
So a DSA switch with 3 interfaces will result in a usage count elevated
by two, and netdev_wait_allrefs will wait until they have gone away.
Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
delete themselves, but DSA cannot just vanish and go poof, at most it
can unbind itself from the switch devices, but that must happen strictly
earlier compared to when the DSA master unregisters its net_device, so
reacting on the NETDEV_UNREGISTER event is way too late.
It seems that it is a pretty established pattern to have a driver's
->shutdown hook redirect to its ->remove hook, so the same code is
executed regardless of whether the driver is unbound from the device, or
the system is just shutting down. As Florian puts it, it is quite a big
hammer for bcmgenet to unregister its net_device during shutdown, but
having a common code path with the driver unbind helps ensure it is well
tested.
So DSA, for better or for worse, has to live with that and engage in an
arms race of implementing the ->shutdown hook too, from all individual
drivers, and do something sane when paired with masters that unregister
their net_device there. The only sane thing to do, of course, is to
unlink from the master.
However, complications arise really quickly.
The pattern of redirecting ->shutdown to ->remove is not unique to
bcmgenet or even to net_device drivers. In fact, SPI controllers do it
too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
and MDIO controllers do it too (this is something I have not researched
too deeply, but even if this is not the case today, it is certainly
plausible to happen in the future, and must be taken into consideration).
Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
insane implication is that for the exact same DSA switch device, we
might have both ->shutdown and ->remove getting called.
So we need to do something with that insane environment. The pattern
I've come up with is "if this, then not that", so if either ->shutdown
or ->remove gets called, we set the device's drvdata to NULL, and in the
other hook, we check whether the drvdata is NULL and just do nothing.
This is probably not necessary for platform devices, just for devices on
buses, but I would really insist for consistency among drivers, because
when code is copy-pasted, it is not always copy-pasted from the best
sources.
So depending on whether the DSA switch's ->remove or ->shutdown will get
called first, we cannot really guarantee even for the same driver if
rebooting will result in the same code path on all platforms. But
nonetheless, we need to do something minimally reasonable on ->shutdown
too to fix the bug. Of course, the ->remove will do more (a full
teardown of the tree, with all data structures freed, and this is why
the bug was not caught for so long). The new ->shutdown method is kept
separate from dsa_unregister_switch not because we couldn't have
unregistered the switch, but simply in the interest of doing something
quick and to the point.
The big question is: does the DSA switch's ->shutdown get called earlier
than the DSA master's ->shutdown? If not, there is still a risk that we
might still trigger the WARN_ON in unregister_netdevice that says we are
attempting to unregister a net_device which has uppers. That's no good.
Although the reference to the master net_device won't physically go away
even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
on it.
The answer to that question lies in this comment above device_link_add:
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
* on it to the ends of these lists (that does not happen to devices that have
* not been registered when this function is called).
so the fact that DSA uses device_link_add towards its master is not
exactly for nothing. device_shutdown() walks devices_kset from the back,
so this is our guarantee that DSA's shutdown happens before the master's
shutdown.
Fixes: 2f1e8ea726e9 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/
Reported-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-17 16:34:33 +03:00
2022-01-28 03:04:59 -03:00
if ( ! priv )
net: dsa: be compatible with masters which unregister on shutdown
Lino reports that on his system with bcmgenet as DSA master and KSZ9897
as a switch, rebooting or shutting down never works properly.
What does the bcmgenet driver have special to trigger this, that other
DSA masters do not? It has an implementation of ->shutdown which simply
calls its ->remove implementation. Otherwise said, it unregisters its
network interface on shutdown.
This message can be seen in a loop, and it hangs the reboot process there:
unregister_netdevice: waiting for eth0 to become free. Usage count = 3
So why 3?
A usage count of 1 is normal for a registered network interface, and any
virtual interface which links itself as an upper of that will increment
it via dev_hold. In the case of DSA, this is the call path:
dsa_slave_create
-> netdev_upper_dev_link
-> __netdev_upper_dev_link
-> __netdev_adjacent_dev_insert
-> dev_hold
So a DSA switch with 3 interfaces will result in a usage count elevated
by two, and netdev_wait_allrefs will wait until they have gone away.
Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
delete themselves, but DSA cannot just vanish and go poof, at most it
can unbind itself from the switch devices, but that must happen strictly
earlier compared to when the DSA master unregisters its net_device, so
reacting on the NETDEV_UNREGISTER event is way too late.
It seems that it is a pretty established pattern to have a driver's
->shutdown hook redirect to its ->remove hook, so the same code is
executed regardless of whether the driver is unbound from the device, or
the system is just shutting down. As Florian puts it, it is quite a big
hammer for bcmgenet to unregister its net_device during shutdown, but
having a common code path with the driver unbind helps ensure it is well
tested.
So DSA, for better or for worse, has to live with that and engage in an
arms race of implementing the ->shutdown hook too, from all individual
drivers, and do something sane when paired with masters that unregister
their net_device there. The only sane thing to do, of course, is to
unlink from the master.
However, complications arise really quickly.
The pattern of redirecting ->shutdown to ->remove is not unique to
bcmgenet or even to net_device drivers. In fact, SPI controllers do it
too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
and MDIO controllers do it too (this is something I have not researched
too deeply, but even if this is not the case today, it is certainly
plausible to happen in the future, and must be taken into consideration).
Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
insane implication is that for the exact same DSA switch device, we
might have both ->shutdown and ->remove getting called.
So we need to do something with that insane environment. The pattern
I've come up with is "if this, then not that", so if either ->shutdown
or ->remove gets called, we set the device's drvdata to NULL, and in the
other hook, we check whether the drvdata is NULL and just do nothing.
This is probably not necessary for platform devices, just for devices on
buses, but I would really insist for consistency among drivers, because
when code is copy-pasted, it is not always copy-pasted from the best
sources.
So depending on whether the DSA switch's ->remove or ->shutdown will get
called first, we cannot really guarantee even for the same driver if
rebooting will result in the same code path on all platforms. But
nonetheless, we need to do something minimally reasonable on ->shutdown
too to fix the bug. Of course, the ->remove will do more (a full
teardown of the tree, with all data structures freed, and this is why
the bug was not caught for so long). The new ->shutdown method is kept
separate from dsa_unregister_switch not because we couldn't have
unregistered the switch, but simply in the interest of doing something
quick and to the point.
The big question is: does the DSA switch's ->shutdown get called earlier
than the DSA master's ->shutdown? If not, there is still a risk that we
might still trigger the WARN_ON in unregister_netdevice that says we are
attempting to unregister a net_device which has uppers. That's no good.
Although the reference to the master net_device won't physically go away
even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
on it.
The answer to that question lies in this comment above device_link_add:
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
* on it to the ends of these lists (that does not happen to devices that have
* not been registered when this function is called).
so the fact that DSA uses device_link_add towards its master is not
exactly for nothing. device_shutdown() walks devices_kset from the back,
so this is our guarantee that DSA's shutdown happens before the master's
shutdown.
Fixes: 2f1e8ea726e9 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/
Reported-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-17 16:34:33 +03:00
return 0 ;
2018-07-14 11:45:55 +02:00
2022-01-28 03:04:59 -03:00
dsa_unregister_switch ( priv - > ds ) ;
if ( priv - > slave_mii_bus )
of_node_put ( priv - > slave_mii_bus - > dev . of_node ) ;
2022-02-13 23:20:11 -03:00
/* leave the device reset asserted */
if ( priv - > reset )
gpiod_set_value ( priv - > reset , 1 ) ;
2018-07-14 11:45:55 +02:00
return 0 ;
}
net: dsa: be compatible with masters which unregister on shutdown
Lino reports that on his system with bcmgenet as DSA master and KSZ9897
as a switch, rebooting or shutting down never works properly.
What does the bcmgenet driver have special to trigger this, that other
DSA masters do not? It has an implementation of ->shutdown which simply
calls its ->remove implementation. Otherwise said, it unregisters its
network interface on shutdown.
This message can be seen in a loop, and it hangs the reboot process there:
unregister_netdevice: waiting for eth0 to become free. Usage count = 3
So why 3?
A usage count of 1 is normal for a registered network interface, and any
virtual interface which links itself as an upper of that will increment
it via dev_hold. In the case of DSA, this is the call path:
dsa_slave_create
-> netdev_upper_dev_link
-> __netdev_upper_dev_link
-> __netdev_adjacent_dev_insert
-> dev_hold
So a DSA switch with 3 interfaces will result in a usage count elevated
by two, and netdev_wait_allrefs will wait until they have gone away.
Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
delete themselves, but DSA cannot just vanish and go poof, at most it
can unbind itself from the switch devices, but that must happen strictly
earlier compared to when the DSA master unregisters its net_device, so
reacting on the NETDEV_UNREGISTER event is way too late.
It seems that it is a pretty established pattern to have a driver's
->shutdown hook redirect to its ->remove hook, so the same code is
executed regardless of whether the driver is unbound from the device, or
the system is just shutting down. As Florian puts it, it is quite a big
hammer for bcmgenet to unregister its net_device during shutdown, but
having a common code path with the driver unbind helps ensure it is well
tested.
So DSA, for better or for worse, has to live with that and engage in an
arms race of implementing the ->shutdown hook too, from all individual
drivers, and do something sane when paired with masters that unregister
their net_device there. The only sane thing to do, of course, is to
unlink from the master.
However, complications arise really quickly.
The pattern of redirecting ->shutdown to ->remove is not unique to
bcmgenet or even to net_device drivers. In fact, SPI controllers do it
too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
and MDIO controllers do it too (this is something I have not researched
too deeply, but even if this is not the case today, it is certainly
plausible to happen in the future, and must be taken into consideration).
Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
insane implication is that for the exact same DSA switch device, we
might have both ->shutdown and ->remove getting called.
So we need to do something with that insane environment. The pattern
I've come up with is "if this, then not that", so if either ->shutdown
or ->remove gets called, we set the device's drvdata to NULL, and in the
other hook, we check whether the drvdata is NULL and just do nothing.
This is probably not necessary for platform devices, just for devices on
buses, but I would really insist for consistency among drivers, because
when code is copy-pasted, it is not always copy-pasted from the best
sources.
So depending on whether the DSA switch's ->remove or ->shutdown will get
called first, we cannot really guarantee even for the same driver if
rebooting will result in the same code path on all platforms. But
nonetheless, we need to do something minimally reasonable on ->shutdown
too to fix the bug. Of course, the ->remove will do more (a full
teardown of the tree, with all data structures freed, and this is why
the bug was not caught for so long). The new ->shutdown method is kept
separate from dsa_unregister_switch not because we couldn't have
unregistered the switch, but simply in the interest of doing something
quick and to the point.
The big question is: does the DSA switch's ->shutdown get called earlier
than the DSA master's ->shutdown? If not, there is still a risk that we
might still trigger the WARN_ON in unregister_netdevice that says we are
attempting to unregister a net_device which has uppers. That's no good.
Although the reference to the master net_device won't physically go away
even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
on it.
The answer to that question lies in this comment above device_link_add:
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
* on it to the ends of these lists (that does not happen to devices that have
* not been registered when this function is called).
so the fact that DSA uses device_link_add towards its master is not
exactly for nothing. device_shutdown() walks devices_kset from the back,
so this is our guarantee that DSA's shutdown happens before the master's
shutdown.
Fixes: 2f1e8ea726e9 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/
Reported-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-17 16:34:33 +03:00
static void realtek_smi_shutdown ( struct platform_device * pdev )
{
2022-01-28 03:04:59 -03:00
struct realtek_priv * priv = platform_get_drvdata ( pdev ) ;
net: dsa: be compatible with masters which unregister on shutdown
Lino reports that on his system with bcmgenet as DSA master and KSZ9897
as a switch, rebooting or shutting down never works properly.
What does the bcmgenet driver have special to trigger this, that other
DSA masters do not? It has an implementation of ->shutdown which simply
calls its ->remove implementation. Otherwise said, it unregisters its
network interface on shutdown.
This message can be seen in a loop, and it hangs the reboot process there:
unregister_netdevice: waiting for eth0 to become free. Usage count = 3
So why 3?
A usage count of 1 is normal for a registered network interface, and any
virtual interface which links itself as an upper of that will increment
it via dev_hold. In the case of DSA, this is the call path:
dsa_slave_create
-> netdev_upper_dev_link
-> __netdev_upper_dev_link
-> __netdev_adjacent_dev_insert
-> dev_hold
So a DSA switch with 3 interfaces will result in a usage count elevated
by two, and netdev_wait_allrefs will wait until they have gone away.
Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
delete themselves, but DSA cannot just vanish and go poof, at most it
can unbind itself from the switch devices, but that must happen strictly
earlier compared to when the DSA master unregisters its net_device, so
reacting on the NETDEV_UNREGISTER event is way too late.
It seems that it is a pretty established pattern to have a driver's
->shutdown hook redirect to its ->remove hook, so the same code is
executed regardless of whether the driver is unbound from the device, or
the system is just shutting down. As Florian puts it, it is quite a big
hammer for bcmgenet to unregister its net_device during shutdown, but
having a common code path with the driver unbind helps ensure it is well
tested.
So DSA, for better or for worse, has to live with that and engage in an
arms race of implementing the ->shutdown hook too, from all individual
drivers, and do something sane when paired with masters that unregister
their net_device there. The only sane thing to do, of course, is to
unlink from the master.
However, complications arise really quickly.
The pattern of redirecting ->shutdown to ->remove is not unique to
bcmgenet or even to net_device drivers. In fact, SPI controllers do it
too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
and MDIO controllers do it too (this is something I have not researched
too deeply, but even if this is not the case today, it is certainly
plausible to happen in the future, and must be taken into consideration).
Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
insane implication is that for the exact same DSA switch device, we
might have both ->shutdown and ->remove getting called.
So we need to do something with that insane environment. The pattern
I've come up with is "if this, then not that", so if either ->shutdown
or ->remove gets called, we set the device's drvdata to NULL, and in the
other hook, we check whether the drvdata is NULL and just do nothing.
This is probably not necessary for platform devices, just for devices on
buses, but I would really insist for consistency among drivers, because
when code is copy-pasted, it is not always copy-pasted from the best
sources.
So depending on whether the DSA switch's ->remove or ->shutdown will get
called first, we cannot really guarantee even for the same driver if
rebooting will result in the same code path on all platforms. But
nonetheless, we need to do something minimally reasonable on ->shutdown
too to fix the bug. Of course, the ->remove will do more (a full
teardown of the tree, with all data structures freed, and this is why
the bug was not caught for so long). The new ->shutdown method is kept
separate from dsa_unregister_switch not because we couldn't have
unregistered the switch, but simply in the interest of doing something
quick and to the point.
The big question is: does the DSA switch's ->shutdown get called earlier
than the DSA master's ->shutdown? If not, there is still a risk that we
might still trigger the WARN_ON in unregister_netdevice that says we are
attempting to unregister a net_device which has uppers. That's no good.
Although the reference to the master net_device won't physically go away
even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
on it.
The answer to that question lies in this comment above device_link_add:
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
* on it to the ends of these lists (that does not happen to devices that have
* not been registered when this function is called).
so the fact that DSA uses device_link_add towards its master is not
exactly for nothing. device_shutdown() walks devices_kset from the back,
so this is our guarantee that DSA's shutdown happens before the master's
shutdown.
Fixes: 2f1e8ea726e9 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/
Reported-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-17 16:34:33 +03:00
2022-01-28 03:04:59 -03:00
if ( ! priv )
net: dsa: be compatible with masters which unregister on shutdown
Lino reports that on his system with bcmgenet as DSA master and KSZ9897
as a switch, rebooting or shutting down never works properly.
What does the bcmgenet driver have special to trigger this, that other
DSA masters do not? It has an implementation of ->shutdown which simply
calls its ->remove implementation. Otherwise said, it unregisters its
network interface on shutdown.
This message can be seen in a loop, and it hangs the reboot process there:
unregister_netdevice: waiting for eth0 to become free. Usage count = 3
So why 3?
A usage count of 1 is normal for a registered network interface, and any
virtual interface which links itself as an upper of that will increment
it via dev_hold. In the case of DSA, this is the call path:
dsa_slave_create
-> netdev_upper_dev_link
-> __netdev_upper_dev_link
-> __netdev_adjacent_dev_insert
-> dev_hold
So a DSA switch with 3 interfaces will result in a usage count elevated
by two, and netdev_wait_allrefs will wait until they have gone away.
Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
delete themselves, but DSA cannot just vanish and go poof, at most it
can unbind itself from the switch devices, but that must happen strictly
earlier compared to when the DSA master unregisters its net_device, so
reacting on the NETDEV_UNREGISTER event is way too late.
It seems that it is a pretty established pattern to have a driver's
->shutdown hook redirect to its ->remove hook, so the same code is
executed regardless of whether the driver is unbound from the device, or
the system is just shutting down. As Florian puts it, it is quite a big
hammer for bcmgenet to unregister its net_device during shutdown, but
having a common code path with the driver unbind helps ensure it is well
tested.
So DSA, for better or for worse, has to live with that and engage in an
arms race of implementing the ->shutdown hook too, from all individual
drivers, and do something sane when paired with masters that unregister
their net_device there. The only sane thing to do, of course, is to
unlink from the master.
However, complications arise really quickly.
The pattern of redirecting ->shutdown to ->remove is not unique to
bcmgenet or even to net_device drivers. In fact, SPI controllers do it
too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
and MDIO controllers do it too (this is something I have not researched
too deeply, but even if this is not the case today, it is certainly
plausible to happen in the future, and must be taken into consideration).
Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
insane implication is that for the exact same DSA switch device, we
might have both ->shutdown and ->remove getting called.
So we need to do something with that insane environment. The pattern
I've come up with is "if this, then not that", so if either ->shutdown
or ->remove gets called, we set the device's drvdata to NULL, and in the
other hook, we check whether the drvdata is NULL and just do nothing.
This is probably not necessary for platform devices, just for devices on
buses, but I would really insist for consistency among drivers, because
when code is copy-pasted, it is not always copy-pasted from the best
sources.
So depending on whether the DSA switch's ->remove or ->shutdown will get
called first, we cannot really guarantee even for the same driver if
rebooting will result in the same code path on all platforms. But
nonetheless, we need to do something minimally reasonable on ->shutdown
too to fix the bug. Of course, the ->remove will do more (a full
teardown of the tree, with all data structures freed, and this is why
the bug was not caught for so long). The new ->shutdown method is kept
separate from dsa_unregister_switch not because we couldn't have
unregistered the switch, but simply in the interest of doing something
quick and to the point.
The big question is: does the DSA switch's ->shutdown get called earlier
than the DSA master's ->shutdown? If not, there is still a risk that we
might still trigger the WARN_ON in unregister_netdevice that says we are
attempting to unregister a net_device which has uppers. That's no good.
Although the reference to the master net_device won't physically go away
even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
on it.
The answer to that question lies in this comment above device_link_add:
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
* on it to the ends of these lists (that does not happen to devices that have
* not been registered when this function is called).
so the fact that DSA uses device_link_add towards its master is not
exactly for nothing. device_shutdown() walks devices_kset from the back,
so this is our guarantee that DSA's shutdown happens before the master's
shutdown.
Fixes: 2f1e8ea726e9 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/
Reported-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-17 16:34:33 +03:00
return ;
2022-01-28 03:04:59 -03:00
dsa_switch_shutdown ( priv - > ds ) ;
net: dsa: be compatible with masters which unregister on shutdown
Lino reports that on his system with bcmgenet as DSA master and KSZ9897
as a switch, rebooting or shutting down never works properly.
What does the bcmgenet driver have special to trigger this, that other
DSA masters do not? It has an implementation of ->shutdown which simply
calls its ->remove implementation. Otherwise said, it unregisters its
network interface on shutdown.
This message can be seen in a loop, and it hangs the reboot process there:
unregister_netdevice: waiting for eth0 to become free. Usage count = 3
So why 3?
A usage count of 1 is normal for a registered network interface, and any
virtual interface which links itself as an upper of that will increment
it via dev_hold. In the case of DSA, this is the call path:
dsa_slave_create
-> netdev_upper_dev_link
-> __netdev_upper_dev_link
-> __netdev_adjacent_dev_insert
-> dev_hold
So a DSA switch with 3 interfaces will result in a usage count elevated
by two, and netdev_wait_allrefs will wait until they have gone away.
Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
delete themselves, but DSA cannot just vanish and go poof, at most it
can unbind itself from the switch devices, but that must happen strictly
earlier compared to when the DSA master unregisters its net_device, so
reacting on the NETDEV_UNREGISTER event is way too late.
It seems that it is a pretty established pattern to have a driver's
->shutdown hook redirect to its ->remove hook, so the same code is
executed regardless of whether the driver is unbound from the device, or
the system is just shutting down. As Florian puts it, it is quite a big
hammer for bcmgenet to unregister its net_device during shutdown, but
having a common code path with the driver unbind helps ensure it is well
tested.
So DSA, for better or for worse, has to live with that and engage in an
arms race of implementing the ->shutdown hook too, from all individual
drivers, and do something sane when paired with masters that unregister
their net_device there. The only sane thing to do, of course, is to
unlink from the master.
However, complications arise really quickly.
The pattern of redirecting ->shutdown to ->remove is not unique to
bcmgenet or even to net_device drivers. In fact, SPI controllers do it
too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
and MDIO controllers do it too (this is something I have not researched
too deeply, but even if this is not the case today, it is certainly
plausible to happen in the future, and must be taken into consideration).
Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
insane implication is that for the exact same DSA switch device, we
might have both ->shutdown and ->remove getting called.
So we need to do something with that insane environment. The pattern
I've come up with is "if this, then not that", so if either ->shutdown
or ->remove gets called, we set the device's drvdata to NULL, and in the
other hook, we check whether the drvdata is NULL and just do nothing.
This is probably not necessary for platform devices, just for devices on
buses, but I would really insist for consistency among drivers, because
when code is copy-pasted, it is not always copy-pasted from the best
sources.
So depending on whether the DSA switch's ->remove or ->shutdown will get
called first, we cannot really guarantee even for the same driver if
rebooting will result in the same code path on all platforms. But
nonetheless, we need to do something minimally reasonable on ->shutdown
too to fix the bug. Of course, the ->remove will do more (a full
teardown of the tree, with all data structures freed, and this is why
the bug was not caught for so long). The new ->shutdown method is kept
separate from dsa_unregister_switch not because we couldn't have
unregistered the switch, but simply in the interest of doing something
quick and to the point.
The big question is: does the DSA switch's ->shutdown get called earlier
than the DSA master's ->shutdown? If not, there is still a risk that we
might still trigger the WARN_ON in unregister_netdevice that says we are
attempting to unregister a net_device which has uppers. That's no good.
Although the reference to the master net_device won't physically go away
even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
on it.
The answer to that question lies in this comment above device_link_add:
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
* on it to the ends of these lists (that does not happen to devices that have
* not been registered when this function is called).
so the fact that DSA uses device_link_add towards its master is not
exactly for nothing. device_shutdown() walks devices_kset from the back,
so this is our guarantee that DSA's shutdown happens before the master's
shutdown.
Fixes: 2f1e8ea726e9 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/
Reported-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-17 16:34:33 +03:00
platform_set_drvdata ( pdev , NULL ) ;
}
2018-07-14 11:45:55 +02:00
static const struct of_device_id realtek_smi_of_match [ ] = {
2022-01-28 03:05:01 -03:00
# if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8366RB)
2018-07-14 11:45:55 +02:00
{
. compatible = " realtek,rtl8366rb " ,
. data = & rtl8366rb_variant ,
} ,
2022-01-28 03:05:01 -03:00
# endif
# if IS_ENABLED(CONFIG_NET_DSA_REALTEK_RTL8365MB)
net: dsa: realtek-smi: add rtl8365mb subdriver for RTL8365MB-VC
This patch adds a realtek-smi subdriver for the RTL8365MB-VC 4+1 port
10/100/1000M switch controller. The driver has been developed based on a
GPL-licensed OS-agnostic Realtek vendor driver known as rtl8367c found
in the OpenWrt source tree.
Despite the name, the RTL8365MB-VC has an entirely different register
layout to the already-supported RTL8366RB ASIC. Notwithstanding this,
the structure of the rtl8365mb subdriver is loosely based on the rtl8366rb
subdriver. Like the 'rb, it establishes its own irqchip to handle
cascaded PHY link status interrupts.
The RTL8365MB-VC switch is capable of offloading a large number of
features from the software, but this patch introduces only the most
basic DSA driver functionality. The ports always function as standalone
ports, with bridging handled in software.
One more thing. Realtek's nomenclature for switches makes it hard to
know exactly what other ASICs might be supported by this driver. The
vendor driver goes by the name rtl8367c, but as far as I can tell, no
chip actually exists under this name. As such, the subdriver is named
rtl8365mb to emphasize the potentially limited support. But it is clear
from the vendor sources that a number of other more advanced switches
share a similar register layout, and further support should not be too
hard to add given access to the relevant hardware. With this in mind,
the subdriver has been written with as few assumptions about the
particular chip as is reasonable. But the RTL8365MB-VC is the only
hardware I have available, so some further work is surely needed.
Co-developed-by: Michael Rasmussen <mir@bang-olufsen.dk>
Signed-off-by: Michael Rasmussen <mir@bang-olufsen.dk>
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Arınç ÜNAL <arinc.unal@arinc9.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-10-18 11:38:01 +02:00
{
. compatible = " realtek,rtl8365mb " ,
. data = & rtl8365mb_variant ,
} ,
2022-01-28 03:05:01 -03:00
# endif
2018-07-14 11:45:55 +02:00
{ /* sentinel */ } ,
} ;
MODULE_DEVICE_TABLE ( of , realtek_smi_of_match ) ;
static struct platform_driver realtek_smi_driver = {
. driver = {
. name = " realtek-smi " ,
. of_match_table = of_match_ptr ( realtek_smi_of_match ) ,
} ,
. probe = realtek_smi_probe ,
. remove = realtek_smi_remove ,
net: dsa: be compatible with masters which unregister on shutdown
Lino reports that on his system with bcmgenet as DSA master and KSZ9897
as a switch, rebooting or shutting down never works properly.
What does the bcmgenet driver have special to trigger this, that other
DSA masters do not? It has an implementation of ->shutdown which simply
calls its ->remove implementation. Otherwise said, it unregisters its
network interface on shutdown.
This message can be seen in a loop, and it hangs the reboot process there:
unregister_netdevice: waiting for eth0 to become free. Usage count = 3
So why 3?
A usage count of 1 is normal for a registered network interface, and any
virtual interface which links itself as an upper of that will increment
it via dev_hold. In the case of DSA, this is the call path:
dsa_slave_create
-> netdev_upper_dev_link
-> __netdev_upper_dev_link
-> __netdev_adjacent_dev_insert
-> dev_hold
So a DSA switch with 3 interfaces will result in a usage count elevated
by two, and netdev_wait_allrefs will wait until they have gone away.
Other stacked interfaces, like VLAN, watch NETDEV_UNREGISTER events and
delete themselves, but DSA cannot just vanish and go poof, at most it
can unbind itself from the switch devices, but that must happen strictly
earlier compared to when the DSA master unregisters its net_device, so
reacting on the NETDEV_UNREGISTER event is way too late.
It seems that it is a pretty established pattern to have a driver's
->shutdown hook redirect to its ->remove hook, so the same code is
executed regardless of whether the driver is unbound from the device, or
the system is just shutting down. As Florian puts it, it is quite a big
hammer for bcmgenet to unregister its net_device during shutdown, but
having a common code path with the driver unbind helps ensure it is well
tested.
So DSA, for better or for worse, has to live with that and engage in an
arms race of implementing the ->shutdown hook too, from all individual
drivers, and do something sane when paired with masters that unregister
their net_device there. The only sane thing to do, of course, is to
unlink from the master.
However, complications arise really quickly.
The pattern of redirecting ->shutdown to ->remove is not unique to
bcmgenet or even to net_device drivers. In fact, SPI controllers do it
too (see dspi_shutdown -> dspi_remove), and presumably, I2C controllers
and MDIO controllers do it too (this is something I have not researched
too deeply, but even if this is not the case today, it is certainly
plausible to happen in the future, and must be taken into consideration).
Since DSA switches might be SPI devices, I2C devices, MDIO devices, the
insane implication is that for the exact same DSA switch device, we
might have both ->shutdown and ->remove getting called.
So we need to do something with that insane environment. The pattern
I've come up with is "if this, then not that", so if either ->shutdown
or ->remove gets called, we set the device's drvdata to NULL, and in the
other hook, we check whether the drvdata is NULL and just do nothing.
This is probably not necessary for platform devices, just for devices on
buses, but I would really insist for consistency among drivers, because
when code is copy-pasted, it is not always copy-pasted from the best
sources.
So depending on whether the DSA switch's ->remove or ->shutdown will get
called first, we cannot really guarantee even for the same driver if
rebooting will result in the same code path on all platforms. But
nonetheless, we need to do something minimally reasonable on ->shutdown
too to fix the bug. Of course, the ->remove will do more (a full
teardown of the tree, with all data structures freed, and this is why
the bug was not caught for so long). The new ->shutdown method is kept
separate from dsa_unregister_switch not because we couldn't have
unregistered the switch, but simply in the interest of doing something
quick and to the point.
The big question is: does the DSA switch's ->shutdown get called earlier
than the DSA master's ->shutdown? If not, there is still a risk that we
might still trigger the WARN_ON in unregister_netdevice that says we are
attempting to unregister a net_device which has uppers. That's no good.
Although the reference to the master net_device won't physically go away
even if DSA's ->shutdown comes afterwards, remember we have a dev_hold
on it.
The answer to that question lies in this comment above device_link_add:
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
* on it to the ends of these lists (that does not happen to devices that have
* not been registered when this function is called).
so the fact that DSA uses device_link_add towards its master is not
exactly for nothing. device_shutdown() walks devices_kset from the back,
so this is our guarantee that DSA's shutdown happens before the master's
shutdown.
Fixes: 2f1e8ea726e9 ("net: dsa: link interfaces with the DSA master to get rid of lockdep warnings")
Link: https://lore.kernel.org/netdev/20210909095324.12978-1-LinoSanfilippo@gmx.de/
Reported-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Tested-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
2021-09-17 16:34:33 +03:00
. shutdown = realtek_smi_shutdown ,
2018-07-14 11:45:55 +02:00
} ;
module_platform_driver ( realtek_smi_driver ) ;
2018-07-20 09:16:02 -07:00
2022-01-28 03:05:01 -03:00
MODULE_AUTHOR ( " Linus Walleij <linus.walleij@linaro.org> " ) ;
MODULE_DESCRIPTION ( " Driver for Realtek ethernet switch connected via SMI interface " ) ;
2018-07-20 09:16:02 -07:00
MODULE_LICENSE ( " GPL " ) ;