Re: [PATCH net-next 4/4] net-next: dsa: add dsa support for Mediatek MT7530 switch
Hi Sean,
[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.11-rc2 next-20170310]
[cannot apply to net-next/master net/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/sean-wang-mediatek-com/dt-bindings-net-dsa-add-Mediatek-MT7530-binding/20170315-083834
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/net/dsa/mt7530.c: In function 'mt7530_probe':
drivers/net/dsa/mt7530.c:1076:27: warning: unused variable 'mdio'
[-Wunused-variable]
struct device_node *dn, *mdio;
^~~~
drivers/net/dsa/mt7530.c: In function 'mt7530_remove':
>> drivers/net/dsa/mt7530.c:1173:9: warning: 'return' with a value, in function
>> returning void
return ret;
^~~
drivers/net/dsa/mt7530.c:1153:1: note: declared here
mt7530_remove(struct mdio_device *mdiodev)
^
vim +/return +1173 drivers/net/dsa/mt7530.c
1070 };
1071
1072 static int
1073 mt7530_probe(struct mdio_device *mdiodev)
1074 {
1075 struct mt7530_priv *priv;
> 1076 struct device_node *dn, *mdio;
1077 int ret;
1078 const char *pm;
1079
1080 dn = mdiodev->dev.of_node;
1081
1082 priv = devm_kzalloc(&mdiodev->dev, sizeof(*priv), GFP_KERNEL);
1083 if (!priv)
1084 return -ENOMEM;
1085
1086 priv->ds = devm_kzalloc(&mdiodev->dev, sizeof(*priv->ds),
GFP_KERNEL);
1087 if (!priv->ds)
1088 return -ENOMEM;
1089
1090 /* Use medatek,mcm property to distinguish hardware type that
would
1091 * casues a little bit differences on power-on sequence.
1092 */
1093 ret = of_property_read_string(dn, "mediatek,mcm", &pm);
1094 if (!ret && !strcasecmp(pm, "enabled")) {
1095 priv->mcm = true;
1096 dev_info(&mdiodev->dev, "MT7530 adapts as multi-chip
module\n");
1097 }
1098
1099 priv->core_pwr = devm_regulator_get(&mdiodev->dev, "core");
1100 if (IS_ERR(priv->core_pwr))
1101 return PTR_ERR(priv->core_pwr);
1102
1103 priv->io_pwr = devm_regulator_get(&mdiodev->dev, "io");
1104 if (IS_ERR(priv->io_pwr))
1105 return PTR_ERR(priv->io_pwr);
1106
1107 /* MT7530 shares the certain address space with Mediatek
Ethernet
1108 * driver for controling TRGMII. Here we create syscon regmap
for
1109 * access and control these parameters up on TRGMII.
1110 */
priv->ethsys = syscon_regmap_lookup_by_phandle(dn,
1112
"mediatek,ethsys");
1113 if (IS_ERR(priv->ethsys))
1114 return PTR_ERR(priv->ethsys);
1115
1116 priv->ethernet = syscon_regmap_lookup_by_phandle(dn,
1117
"mediatek,ethernet");
1118 if (IS_ERR(priv->ethernet))
1119 return PTR_ERR(priv->ethernet);
1120
1121 /* Not MCM that indicates switch works as the remote standalone
1122 * integrated circuit so the GPIO pin would be used to complete
1123 * the reset, otherwise memory-mapped register accessing used
1124 * through syscon provides in the case of MCM.
1125 */
1126 if (!priv->mcm) {
1127 priv->reset = of_get_named_gpio(dn,
"mediatek,reset-pin", 0);
1128 if (!gpio_is_valid(priv->reset))
1129 return priv->reset;
1130
1131 ret = devm_gpio_request_one(&mdiodev->dev,
1132 priv->reset,
GPIOF_OUT_INIT_LOW,
1133 "mediatek,reset-pin");
1134 if (ret < 0) {
1135 dev_err(&mdiodev->dev,
1136 "fail to devm_gpio_request reset\n");
1137 return ret;
1138 }
1139 }
1140
1141 priv->bus = mdiodev->bus;
1142 priv->dev = &mdiodev->dev;
1143 priv->ds->priv = priv;
1144 priv->ds->dev = &mdiodev->dev;
1145 priv->ds->ops = &mt7530_switch_ops;
1146 mutex_init(&priv->reg_mutex);
1147 dev_set_drvdata(&mdiodev->dev, priv);
1148
1149 return dsa_register_switch(priv->ds, priv->ds->dev->of_node);
1150 }
1151
1152
Re: [PATCH net-next 4/4] net-next: dsa: add dsa support for Mediatek MT7530 switch
Hi Sean,
[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.11-rc2 next-20170310]
[cannot apply to net-next/master net/master]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/sean-wang-mediatek-com/dt-bindings-net-dsa-add-Mediatek-MT7530-binding/20170315-083834
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from drivers/net/dsa/mt7530.c:25:0:
drivers/net/dsa/mt7530.c: In function 'mt7530_setup':
>> drivers/net/dsa/mt7530.c:699:19: warning: large integer implicitly truncated
>> to unsigned type [-Woverflow]
RESET_MCM, ~RESET_MCM);
^
include/linux/regmap.h:70:42: note: in definition of macro
'regmap_update_bits'
regmap_update_bits_base(map, reg, mask, val, NULL, false, false)
^~~
drivers/net/dsa/mt7530.c: In function 'mt7530_probe':
drivers/net/dsa/mt7530.c:1076:27: warning: unused variable 'mdio'
[-Wunused-variable]
struct device_node *dn, *mdio;
^~~~
drivers/net/dsa/mt7530.c: In function 'mt7530_remove':
drivers/net/dsa/mt7530.c:1173:9: warning: 'return' with a value, in function
returning void
return ret;
^~~
drivers/net/dsa/mt7530.c:1153:1: note: declared here
mt7530_remove(struct mdio_device *mdiodev)
^
vim +699 drivers/net/dsa/mt7530.c
683 ret = regulator_enable(priv->io_pwr);
684 if (ret < 0) {
685 dev_err(priv->dev, "Failed to enable io pwr: %d\n",
686 ret);
687 return ret;
688 }
689
690 /* Reset whole chip through gpio pin or
691 * memory-mapped registers for different
692 * type of hardware
693 */
694 if (priv->mcm) {
695 regmap_update_bits(priv->ethsys, SYSC_REG_RSTCTRL,
696 RESET_MCM, RESET_MCM);
697 usleep_range(1000, 1100);
698 regmap_update_bits(priv->ethsys, SYSC_REG_RSTCTRL,
> 699 RESET_MCM, ~RESET_MCM);
700 } else {
701 gpio_direction_output(priv->reset, 0);
702 usleep_range(1000, 1100);
703 gpio_set_value(priv->reset, 1);
704 }
705
706 /* Wait until the reset completion */
707 ret = wait_condition_timeout(mt7530_read(priv, MT7530_HWTRAP)
!= 0,
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH net-next 4/4] net-next: dsa: add dsa support for Mediatek MT7530 switch
On Tue, 2017-03-14 at 00:11 +0100, Andrew Lunn wrote:
> > +static int
> > +mt7530_setup(struct dsa_switch *ds)
> > +{
> > + struct mt7530_priv *priv = ds->priv;
> > + int ret, i, phy_mode;
> > + u8 cpup_mask = 0;
> > + u32 id, val;
> > + struct regmap *regmap;
> > +
> > + /* Make sure that cpu port specfied on the dt is appropriate */
> > + if (!dsa_is_cpu_port(ds, MT7530_CPU_PORT)) {
> > + dev_err(priv->dev, "port not matched with the CPU port\n");
> > + return -EINVAL;
> > + }
> > +
> > + regmap = devm_regmap_init(ds->dev, NULL, priv,
> > + &mt7530_regmap_config);
> > + if (IS_ERR(regmap))
> > + dev_warn(priv->dev, "phy regmap initialization failed");
> > +
> > + phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> > + if (phy_mode < 0) {
> > + dev_err(priv->dev, "Can't find phy-mode for master device\n");
> > + return phy_mode;
> > + }
> > + dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);
>
> Hi Sean
>
> It is not documented in the binding that a phy-mode is mandatory for
> the cpu port.
>
> Andrew
Hi Andrew,
thanks for your reviewing. I'll also add the missing part into the next
one.
Sean
Re: [PATCH net-next 4/4] net-next: dsa: add dsa support for Mediatek MT7530 switch
> +static int
> +mt7530_setup(struct dsa_switch *ds)
> +{
> + struct mt7530_priv *priv = ds->priv;
> + int ret, i, phy_mode;
> + u8 cpup_mask = 0;
> + u32 id, val;
> + struct regmap *regmap;
> +
> + /* Make sure that cpu port specfied on the dt is appropriate */
> + if (!dsa_is_cpu_port(ds, MT7530_CPU_PORT)) {
> + dev_err(priv->dev, "port not matched with the CPU port\n");
> + return -EINVAL;
> + }
> +
> + regmap = devm_regmap_init(ds->dev, NULL, priv,
> + &mt7530_regmap_config);
> + if (IS_ERR(regmap))
> + dev_warn(priv->dev, "phy regmap initialization failed");
> +
> + phy_mode = of_get_phy_mode(ds->ports[ds->dst->cpu_port].dn);
> + if (phy_mode < 0) {
> + dev_err(priv->dev, "Can't find phy-mode for master device\n");
> + return phy_mode;
> + }
> + dev_info(priv->dev, "phy-mode for master device = %x\n", phy_mode);
Hi Sean
It is not documented in the binding that a phy-mode is mandatory for
the cpu port.
Andrew
Re: [PATCH net-next 4/4] net-next: dsa: add dsa support for Mediatek MT7530 switch
Hi Sean
Just looking at the GPIO handling at the moment.
> + /* Reset whole chip through gpio pin or
> + * memory-mapped registers for different
> + * type of hardware
> + */
> + if (priv->mcm) {
> + regmap_update_bits(priv->ethsys, SYSC_REG_RSTCTRL,
> +RESET_MCM, RESET_MCM);
> + usleep_range(1000, 1100);
> + regmap_update_bits(priv->ethsys, SYSC_REG_RSTCTRL,
> +RESET_MCM, ~RESET_MCM);
> + } else {
> + gpio_direction_output(priv->reset, 0);
> + usleep_range(1000, 1100);
> + gpio_set_value(priv->reset, 1);
> + }
> + /* Not MCM that indicates switch works as the remote standalone
> + * integrated circuit so the GPIO pin would be used to complete
> + * the reset, otherwise memory-mapped register accessing used
> + * through syscon provides in the case of MCM.
> + */
> + if (!priv->mcm) {
> + priv->reset = of_get_named_gpio(dn, "mediatek,reset-pin", 0);
> + if (!gpio_is_valid(priv->reset))
> + return priv->reset;
> +
> + ret = devm_gpio_request_one(&mdiodev->dev,
> + priv->reset, GPIOF_OUT_INIT_LOW,
> + "mediatek,reset-pin");
> + if (ret < 0) {
> + dev_err(&mdiodev->dev,
> + "fail to devm_gpio_request reset\n");
> + return ret;
> + }
> + }
You are not handling the flags part of the GPIO binding. It is better
to use devm_gpiod_ API calls, which will handle the active low flags
for you.
Andrew
