| Message ID | 20191014074618.15506-2-joel@jms.id.au |
|---|---|
| State | Rejected, archived |
| Headers | show |
| Series | Tacoma FSI GPIOs | expand |
On Mon, 14 Oct 2019, at 18:16, Joel Stanley wrote: > On Tacoma the enable and mux GPIOs allow the BMC to enable FSI and > control if the clock and data should come from the BMC, or from the > debug connector. > > Configure these as optional GPIOs which will fail without error for > systems that do not have these GPIOs in their device tree. > > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/fsi/fsi-master-aspeed.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c > index 6496d2972944..2048e38e7200 100644 > --- a/drivers/fsi/fsi-master-aspeed.c > +++ b/drivers/fsi/fsi-master-aspeed.c > @@ -14,6 +14,7 @@ > #include <linux/regmap.h> > #include <linux/slab.h> > #include <linux/iopoll.h> > +#include <linux/gpio/consumer.h> > > #include "fsi-master.h" > > @@ -85,6 +86,9 @@ struct fsi_master_aspeed { > void __iomem *base; > struct clk *clk; > > + struct gpio_desc *gpio_enable; > + struct gpio_desc *gpio_mux; > + > struct dentry *debugfs_dir; > struct fsi_master_aspeed_debugfs_entry debugfs[FSI_NUM_DEBUGFS_ENTRIES]; > }; > @@ -471,6 +475,30 @@ DEFINE_DEBUGFS_ATTRIBUTE(fsi_master_aspeed_debugfs_ops, > fsi_master_aspeed_debugfs_get, > fsi_master_aspeed_debugfs_set, "0x%08llx\n"); > > +static int aspeed_master_gpio_init(struct fsi_master_aspeed *master) > +{ > + struct device *dev = master->dev; > + struct gpio_desc *gpio; > + > + gpio = devm_gpiod_get_optional(dev, "fsi-enable", 0); > + if (IS_ERR(gpio)) { devm_gpiod_get_option() returns NULL if the GPIO isn't defined, and interfaces like gpiod_direction_output(...) return 0 if the descriptor is NULL. The design point of the _optional() interfaces is that it makes the error handling unnecessary because we know the GPIO might be absent. The pattern is also wrong in fsi-master-ast-cf and probably fsi-master-gpio.
On Tue, 15 Oct 2019, at 10:57, Andrew Jeffery wrote: > > > On Mon, 14 Oct 2019, at 18:16, Joel Stanley wrote: > > On Tacoma the enable and mux GPIOs allow the BMC to enable FSI and > > control if the clock and data should come from the BMC, or from the > > debug connector. > > > > Configure these as optional GPIOs which will fail without error for > > systems that do not have these GPIOs in their device tree. > > > > Signed-off-by: Joel Stanley <joel@jms.id.au> > > --- > > drivers/fsi/fsi-master-aspeed.c | 30 ++++++++++++++++++++++++++++++ > > 1 file changed, 30 insertions(+) > > > > diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c > > index 6496d2972944..2048e38e7200 100644 > > --- a/drivers/fsi/fsi-master-aspeed.c > > +++ b/drivers/fsi/fsi-master-aspeed.c > > @@ -14,6 +14,7 @@ > > #include <linux/regmap.h> > > #include <linux/slab.h> > > #include <linux/iopoll.h> > > +#include <linux/gpio/consumer.h> > > > > #include "fsi-master.h" > > > > @@ -85,6 +86,9 @@ struct fsi_master_aspeed { > > void __iomem *base; > > struct clk *clk; > > > > + struct gpio_desc *gpio_enable; > > + struct gpio_desc *gpio_mux; > > + > > struct dentry *debugfs_dir; > > struct fsi_master_aspeed_debugfs_entry debugfs[FSI_NUM_DEBUGFS_ENTRIES]; > > }; > > @@ -471,6 +475,30 @@ DEFINE_DEBUGFS_ATTRIBUTE(fsi_master_aspeed_debugfs_ops, > > fsi_master_aspeed_debugfs_get, > > fsi_master_aspeed_debugfs_set, "0x%08llx\n"); > > > > +static int aspeed_master_gpio_init(struct fsi_master_aspeed *master) > > +{ > > + struct device *dev = master->dev; > > + struct gpio_desc *gpio; > > + > > + gpio = devm_gpiod_get_optional(dev, "fsi-enable", 0); > > + if (IS_ERR(gpio)) { > > devm_gpiod_get_option() returns NULL if the GPIO isn't defined, and > interfaces like gpiod_direction_output(...) return 0 if the descriptor is > NULL. The design point of the _optional() interfaces is that it makes the > error handling unnecessary because we know the GPIO might be absent. > > The pattern is also wrong in fsi-master-ast-cf and probably fsi-master-gpio. Actually, disregard that, NULL is only returned for -ENOENT but there may be other errors. Sorry for the noise.
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c index 6496d2972944..2048e38e7200 100644 --- a/drivers/fsi/fsi-master-aspeed.c +++ b/drivers/fsi/fsi-master-aspeed.c @@ -14,6 +14,7 @@ #include <linux/regmap.h> #include <linux/slab.h> #include <linux/iopoll.h> +#include <linux/gpio/consumer.h> #include "fsi-master.h" @@ -85,6 +86,9 @@ struct fsi_master_aspeed { void __iomem *base; struct clk *clk; + struct gpio_desc *gpio_enable; + struct gpio_desc *gpio_mux; + struct dentry *debugfs_dir; struct fsi_master_aspeed_debugfs_entry debugfs[FSI_NUM_DEBUGFS_ENTRIES]; }; @@ -471,6 +475,30 @@ DEFINE_DEBUGFS_ATTRIBUTE(fsi_master_aspeed_debugfs_ops, fsi_master_aspeed_debugfs_get, fsi_master_aspeed_debugfs_set, "0x%08llx\n"); +static int aspeed_master_gpio_init(struct fsi_master_aspeed *master) +{ + struct device *dev = master->dev; + struct gpio_desc *gpio; + + gpio = devm_gpiod_get_optional(dev, "fsi-enable", 0); + if (IS_ERR(gpio)) { + dev_err(dev, "failed to get fsi enable gpio\n"); + return PTR_ERR(gpio); + } + master->gpio_enable = gpio; + gpiod_direction_output(master->gpio_enable, 1); + + gpio = devm_gpiod_get_optional(dev, "fsi-mux", 0); + if (IS_ERR(gpio)) { + dev_err(dev, "failed to get fsi mux gpio\n"); + return PTR_ERR(gpio); + } + master->gpio_mux = gpio; + gpiod_direction_output(master->gpio_mux, 1); + + return 0; +} + static int fsi_master_aspeed_probe(struct platform_device *pdev) { struct fsi_master_aspeed *aspeed; @@ -540,6 +568,8 @@ static int fsi_master_aspeed_probe(struct platform_device *pdev) dev_set_drvdata(&pdev->dev, aspeed); + aspeed_master_gpio_init(aspeed); + aspeed_master_init(aspeed); aspeed->debugfs_dir = debugfs_create_dir("fsi-master-aspeed", NULL);
On Tacoma the enable and mux GPIOs allow the BMC to enable FSI and control if the clock and data should come from the BMC, or from the debug connector. Configure these as optional GPIOs which will fail without error for systems that do not have these GPIOs in their device tree. Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/fsi/fsi-master-aspeed.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)