Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1466362 > unrolled thread
| Started by | Andrew Jeffery <andrew@aj.id.au> |
|---|---|
| First post | 2016-08-19 14:50 +0200 |
| Last post | 2016-08-23 04:40 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/8] aspeed: Add pinctrl and gpio drivers Andrew Jeffery <andrew@aj.id.au> - 2016-08-19 14:50 +0200
Re: [PATCH v2 5/8] pinctrl: Add core support for Aspeed SoCs Linus Walleij <linus.walleij@linaro.org> - 2016-08-22 15:50 +0200
Re: [PATCH v2 5/8] pinctrl: Add core support for Aspeed SoCs Andrew Jeffery <andrew@aj.id.au> - 2016-08-23 04:40 +0200
| From | Andrew Jeffery <andrew@aj.id.au> |
|---|---|
| Date | 2016-08-19 14:50 +0200 |
| Subject | [PATCH v2 0/8] aspeed: Add pinctrl and gpio drivers |
| Message-ID | <s7Rpf-5FZ-3@gated-at.bofh.it> |
Hi all, Here's v2 of the Aspeed pinctrl and gpio driver patches, which aims to address the review comments on v1: https://lkml.org/lkml/2016/7/20/69 There are some additional changes which are either to fix bugs with some signal descriptor values or to support additional mux functions. Further, the series has been split in two, into driver and SoC changes. The combined series has been tested with both the AST2400 (g4) and AST2500 (g5) SoCs on OpenPOWER Palmetto and Aspeed AST2500 EVB machines respectively, and similarly in QEMU with the palmetto-bmc and ast2500-evb machines[1]. Cheers, Andrew [1] https://lists.gnu.org/archive/html/qemu-arm/2016-08/msg00019.html Since v1: * Split the series in two: driver and SoC patches. * Update devicetree bindings documentation * Add documentation for aspeed pinctrl core functions * Fix bugs in existing signal descriptors * Add a number of pin and mux function declarations * Provide debug information on unexpected conditions --- Andrew Jeffery (7): MAINTAINERS: Add glob for Aspeed devicetree bindings syscon: dt-bindings: Add documentation for Aspeed system control units pinctrl: dt-bindings: Add documentation for Aspeed pin controllers gpio: dt-bindings: Add documentation for Aspeed GPIO controllers pinctrl: Add core support for Aspeed SoCs pinctrl: Add pinctrl-aspeed-g4 driver pinctrl: Add pinctrl-aspeed-g5 driver Joel Stanley (1): gpio: Add Aspeed driver .../devicetree/bindings/gpio/gpio-aspeed.txt | 34 + .../devicetree/bindings/mfd/aspeed-scu.txt | 18 + .../devicetree/bindings/pinctrl/pinctrl-aspeed.txt | 65 ++ MAINTAINERS | 2 + drivers/gpio/Kconfig | 7 + drivers/gpio/Makefile | 1 + drivers/gpio/gpio-aspeed.c | 457 ++++++++ drivers/pinctrl/Kconfig | 1 + drivers/pinctrl/Makefile | 1 + drivers/pinctrl/aspeed/Kconfig | 24 + drivers/pinctrl/aspeed/Makefile | 6 + drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c | 1231 ++++++++++++++++++++ drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c | 808 +++++++++++++ drivers/pinctrl/aspeed/pinctrl-aspeed.c | 501 ++++++++ drivers/pinctrl/aspeed/pinctrl-aspeed.h | 569 +++++++++ 15 files changed, 3725 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpio/gpio-aspeed.txt create mode 100644 Documentation/devicetree/bindings/mfd/aspeed-scu.txt create mode 100644 Documentation/devicetree/bindings/pinctrl/pinctrl-aspeed.txt create mode 100644 drivers/gpio/gpio-aspeed.c create mode 100644 drivers/pinctrl/aspeed/Kconfig create mode 100644 drivers/pinctrl/aspeed/Makefile create mode 100644 drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c create mode 100644 drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c create mode 100644 drivers/pinctrl/aspeed/pinctrl-aspeed.c create mode 100644 drivers/pinctrl/aspeed/pinctrl-aspeed.h -- 2.9.3
[toc] | [next] | [standalone]
| From | Linus Walleij <linus.walleij@linaro.org> |
|---|---|
| Date | 2016-08-22 15:50 +0200 |
| Subject | Re: [PATCH v2 5/8] pinctrl: Add core support for Aspeed SoCs |
| Message-ID | <s8XLY-6Xy-39@gated-at.bofh.it> |
| In reply to | #1466362 |
On Fri, Aug 19, 2016 at 2:44 PM, Andrew Jeffery <andrew@aj.id.au> wrote:
> +++ b/drivers/pinctrl/aspeed/Kconfig
> @@ -0,0 +1,8 @@
> +config PINCTRL_ASPEED
> + bool
> + depends on (ARCH_ASPEED || COMPILE_TEST) && OF
> + select PINMUX
> + select PINCONF
> + select GENERIC_PINCONF
> + select MFD_SYSCON
> + select REGMAP_MMIO
Since this device is spawn from the syscon, should it not be
"depends on MFD_SYSCON"?
(No big deal, if you think this is right then go with it.)
> +#include <linux/io.h>
What is this include for?
> +#include <linux/mfd/syscon.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include "../core.h"
> +#include "pinctrl-aspeed.h"
No #include <linux/regmap.h>?
Maybe some #includes are centralized to pinctrl-aspeed.h
I don't know, just make sure you don't have implicit includes.
> + if (regmap_read(map, desc->reg, &val) < 0)
> + return false;
> +
> + val &= ~desc->mask;
> + val |= pattern << __ffs(desc->mask);
> +
> + if (regmap_write(map, desc->reg, val) < 0)
> + return false;
Use:
regmap_update_bits(map,
desc->reg,
desc->mask,
pattern << __ffs->desc->mask);
Or something like that instead of reimplementing
mask-and-set. Regmap already knows how to do the
business.
(Applied everywhere in the driver where you have a
mask-and-set like this).
The expression core engine is still a complete mystery
for me, I will just trust you that it works as intended.
Yours,
Linus Walleij
[toc] | [prev] | [next] | [standalone]
| From | Andrew Jeffery <andrew@aj.id.au> |
|---|---|
| Date | 2016-08-23 04:40 +0200 |
| Subject | Re: [PATCH v2 5/8] pinctrl: Add core support for Aspeed SoCs |
| Message-ID | <s99N7-6le-1@gated-at.bofh.it> |
| In reply to | #1467645 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, 2016-08-22 at 15:45 +0200, Linus Walleij wrote: > On Fri, Aug 19, 2016 at 2:44 PM, Andrew Jeffery <andrew@aj.id.au> wrote: > > > > > +++ b/drivers/pinctrl/aspeed/Kconfig > > @@ -0,0 +1,8 @@ > > +config PINCTRL_ASPEED > > + bool > > + depends on (ARCH_ASPEED || COMPILE_TEST) && OF > > + select PINMUX > > + select PINCONF > > + select GENERIC_PINCONF > > + select MFD_SYSCON > > + select REGMAP_MMIO > Since this device is spawn from the syscon, should it not be > "depends on MFD_SYSCON"? > > (No big deal, if you think this is right then go with it.) I think that's a fair point, I will look at rearranging it. > > > > > +#include > What is this include for? Cruft from past iterations. Will remove. > > > > > +#include > > +#include > > +#include > > +#include > > +#include "../core.h" > > +#include "pinctrl-aspeed.h" > No #include ? > > Maybe some #includes are centralized to pinctrl-aspeed.h > I don't know, just make sure you don't have implicit includes. linux/regmap.h is included in pinctrl-aspeed.h > > > > > + if (regmap_read(map, desc->reg, &val) < 0) > > + return false; > > + > > + val &= ~desc->mask; > > + val |= pattern << __ffs(desc->mask); > > + > > + if (regmap_write(map, desc->reg, val) < 0) > > + return false; > Use: > > regmap_update_bits(map, > desc->reg, > desc->mask, > pattern << __ffs->desc->mask); > > Or something like that instead of reimplementing > mask-and-set. Regmap already knows how to do the > business. > > (Applied everywhere in the driver where you have a > mask-and-set like this). Right, I will clean that up. > > The expression core engine is still a complete mystery > for me, I will just trust you that it works as intended. Gah! However, thanks! I will clean up the issues above and re-send. Cheers, Andrew
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web