[RFC] smsc911x device tree binding questions

[Lorenzo Pieralisi]

Hi Steve, all

I am in the process of defining a binding and relative code to parse it,
in order to initialize the smsc911x series of controllers through the device
tree instead of resorting to static data instantiated within the platform
device struct (of course static config will still be there for platforms which
do not rely on device tree probing, no worries, it is not disruptive at all).
I have a couple of questions on the flags passed with the platform_data pointer
(smsc911x_platform_config - flags member) and the way we may encode these bits
of information in the tree:

/* Constants for flags */
#define SMSC911X_USE_16BIT 			(BIT(0))
#define SMSC911X_USE_32BIT 			(BIT(1))
#define SMSC911X_FORCE_INTERNAL_PHY		(BIT(2))
#define SMSC911X_FORCE_EXTERNAL_PHY		(BIT(3))
#define SMSC911X_SAVE_MAC_ADDRESS		(BIT(4))

* 16-32bit bus IF

#define SMSC911X_USE_16BIT		(BIT(0))
#define SMSC911X_USE_32BIT		(BIT(1))

- the SMSC911x host bus size can be retrieved from the chip idrev (for
chip idrev having just 16-bit bus IF) or for controllers like 9118 that
can have both (32/16), there is a D32/D16 HW strap readable from HW_CFG.
The problem with these flags is that they define bus size from a CPU
perspective if I got it right, in particular if the CPU can access
the SMSC with 32-bit load/store to access 32 bit registers or
have to resort to two "locked" 16-bit accesses to ensure
atomicity. I think it is possible on some architectures to use a 32-bit
load/store even if the smsc911x IF is 16-bit (e.g. bus downsize converters,
which ensure 16-bit load/store locking).
It depends on the bus interconnect architecture. I think we
need a property to define this, thoughts much appreciated.

* forcing internal/ext PHY

#define SMSC911X_FORCE_INTERNAL_PHY		(BIT(2))
#define SMSC911X_FORCE_EXTERNAL_PHY 	(BIT(3))

- The device tree allows to describe the mdio bus and all PHYs which are present
on a given controller. Again for controllers having just an internal PHY there
is nothing to force, just use the internal PHY and that's it, that's what
the driver does. If the controller supports both EXT and internal PHY,
since the internal PHY is always at address 0x1 (am I correct ?), we may define 
something like: 
if the tree defines a PHY @ 0x1 ("reg" property) let's use it (force it or at
least higher priority than ext phy if both are defined), otherwise if the tree
defines a PHY which is not @ 0x1 let's use it (force ext).
If no PHY is specified in the tree, let's resort to HW probing (EXT_PHY_DET in 
HW_CFG reg).

All in all, this means: just use the info from the tree, if there are no
PHYs specified resort to HW probing for EXT PHY, if it fails use internal.

Acceptable ?

* save MAC ADDRESS

#define SMSC911X_SAVE_MAC_ADDRESS		(BIT(4))

-  Since we can encode the mac address in the device tree ("local-mac-address" 
property) I am not sure this flag is useful if probing from the tree is enabled.
It is used if the chip is not fitted with an EEPROM, to save the MAC address 
set by a boot-loader before resetting the chip (at probe). Since we can still 
retrieve the MAC from the tree and reprogram it when the content of the 
registers is gone at reset, maybe there is no point in using the option 
(if probing from the tree) or at least no point in defining the flag in
the tree.


Comments very much welcome, thank you very much indeed.

Cheers,
Lorenzo