Message ID | 20220127003318.3633212-3-robert.hancock@calian.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Allow disabling KSZ switch refclock | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Clearly marked for net-next |
netdev/fixes_present | success | Fixes tag not required for -next series |
netdev/subject_prefix | success | Link |
netdev/cover_letter | success | Series has a cover letter |
netdev/patch_count | success | Link |
netdev/header_inline | success | No static functions without inline keyword in header files |
netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
netdev/cc_maintainers | success | CCed 9 of 9 maintainers |
netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
netdev/module_param | success | Was 0 now: 0 |
netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
netdev/verify_fixes | success | No Fixes tag |
netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 34 lines checked |
netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
netdev/source_inline | success | Was 0 now: 0 |
On 1/26/2022 4:33 PM, Robert Hancock wrote: > Add a new microchip,synclko-disable property which can be specified > to disable the reference clock output from the device if not required > by the board design. > > Signed-off-by: Robert Hancock <robert.hancock@calian.com> This looks good, I would just have done the hunk below a bit differently: > --- > drivers/net/dsa/microchip/ksz9477.c | 7 ++++++- > drivers/net/dsa/microchip/ksz_common.c | 6 ++++++ > drivers/net/dsa/microchip/ksz_common.h | 1 + > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > index 353b5f981740..33d52050cd68 100644 > --- a/drivers/net/dsa/microchip/ksz9477.c > +++ b/drivers/net/dsa/microchip/ksz9477.c > @@ -222,9 +222,14 @@ static int ksz9477_reset_switch(struct ksz_device *dev) > (BROADCAST_STORM_VALUE * > BROADCAST_STORM_PROT_RATE) / 100); > > - if (dev->synclko_125) > + if (dev->synclko_disable) > + ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, 0); > + else if (dev->synclko_125) > ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, > SW_ENABLE_REFCLKO | SW_REFCLKO_IS_125MHZ); > + else > + ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, > + SW_ENABLE_REFCLKO); Since you write to the same register in all of these branches, why not do this: u32 tmp = SW_ENABLE_REFCLKO; if (dev->synclko_disable) tmp = 0 else if (dev->synclko_125) tmp = SW_ENABLE_REFCLKO | SW_REFCLKO_IS_125MHZ; ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, tmp); even though the compiler may just do that for you under the hood.
diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c index 353b5f981740..33d52050cd68 100644 --- a/drivers/net/dsa/microchip/ksz9477.c +++ b/drivers/net/dsa/microchip/ksz9477.c @@ -222,9 +222,14 @@ static int ksz9477_reset_switch(struct ksz_device *dev) (BROADCAST_STORM_VALUE * BROADCAST_STORM_PROT_RATE) / 100); - if (dev->synclko_125) + if (dev->synclko_disable) + ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, 0); + else if (dev->synclko_125) ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, SW_ENABLE_REFCLKO | SW_REFCLKO_IS_125MHZ); + else + ksz_write8(dev, REG_SW_GLOBAL_OUTPUT_CTRL__1, + SW_ENABLE_REFCLKO); return 0; } diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 55dbda04ea62..7e33ec73f803 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -434,6 +434,12 @@ int ksz_switch_register(struct ksz_device *dev, } dev->synclko_125 = of_property_read_bool(dev->dev->of_node, "microchip,synclko-125"); + dev->synclko_disable = of_property_read_bool(dev->dev->of_node, + "microchip,synclko-disable"); + if (dev->synclko_125 && dev->synclko_disable) { + dev_err(dev->dev, "inconsistent synclko settings\n"); + return -EINVAL; + } } ret = dsa_register_switch(dev->ds); diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h index df8ae59c8525..3db63f62f0a1 100644 --- a/drivers/net/dsa/microchip/ksz_common.h +++ b/drivers/net/dsa/microchip/ksz_common.h @@ -75,6 +75,7 @@ struct ksz_device { u32 regs_size; bool phy_errata_9477; bool synclko_125; + bool synclko_disable; struct vlan_table *vlan_cache;
Add a new microchip,synclko-disable property which can be specified to disable the reference clock output from the device if not required by the board design. Signed-off-by: Robert Hancock <robert.hancock@calian.com> --- drivers/net/dsa/microchip/ksz9477.c | 7 ++++++- drivers/net/dsa/microchip/ksz_common.c | 6 ++++++ drivers/net/dsa/microchip/ksz_common.h | 1 + 3 files changed, 13 insertions(+), 1 deletion(-)