| Message ID | 20180226130558.7634-1-rf@opensource.cirrus.com |
|---|---|
| Headers | show |
| Series | Add support for Cirrus Logic CS47L35/L85/L90/L91 codecs | expand |
On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > This patch adds a header file of register definitions for Cirrus > Logic "Madera" class codecs. These codecs are all based off a common > set of hardware IP so have a common register map (with a few minor > device-to-device variations). > > The registers.h file is tool-generated directly from the hardware design > but has been manually stripped down to reduce size (full register > map is >44000 lines). All names are kept the same as datasheet names > so that they can be cross-referenced between source and datasheet without > confusion. > > The register map layout is kept fully-defined rather than factored into > macros and/or block-indexing code. The major reasons for this are: > > - #1 is that it makes the source highly greppable, which is important. > "What does the driver do with register bits XYZ" or "Where does it use > register bits XYZ" are commonly types of questions. These can be quickly > answered by a grep. Squashing definitions into generator macros or block- > indexing code is a way of defeating grep. > > - most of the register definitions are used in tables, so a constant value > is required. Using generator macros make the table definition clunky and > obscure. > > - the code is clearer when it's there in the source exactly what register > and field it is using > > - it is easier to diff the register map of a new (unsupported) codec against > what is already supported and merge in differences > > - it makes the register map available in source for maintenance/debugging > instead of having to refer back to the datasheet for a register map ... > +#define MADERA_OTP_HPDET_GRADIENT_1X_MASK 0x0000FF00 > +#define MADERA_OTP_HPDET_GRADIENT_0X_MASK 0x000000FF GENMASK() for all masks?
On Mon, Feb 26, 2018 at 10:05 AM, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > --- /dev/null > +++ b/drivers/mfd/madera-core.c > @@ -0,0 +1,602 @@ > +// SPDX-License-Identifier: GPL-2.0 As you use SPDX line here.... > +/* > + * Core MFD support for Cirrus Logic Madera codecs > + * > + * Copyright (C) 2015-2018 Cirrus Logic > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by the > + * Free Software Foundation; version 2. You can remove this paragraph. Same applies to the other places of this series. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > This adds the generic core support for Cirrus Logic "Madera" class codecs. > These are complex audio codec SoCs with a variety of digital and analogue > I/O, onboard audio processing and DSPs, and other features. > > These codecs are all based off a common set of hardware IP so can be > supported by a core of common code (with a few minor device-to-device > variations). > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by the > + * Free Software Foundation; version 2. This is redundant. > +static void madera_enable_hard_reset(struct madera *madera) > +{ > + if (madera->reset_gpio) if (!...) return > + gpiod_set_value_cansleep(madera->reset_gpio, 0); > +} > + > +static void madera_disable_hard_reset(struct madera *madera) > +{ > + if (madera->reset_gpio) { Ditto. > + gpiod_set_value_cansleep(madera->reset_gpio, 1); > + usleep_range(1000, 2000); > + } > +} > + > +#ifdef CONFIG_PM __maybe_unused > +const struct dev_pm_ops madera_pm_ops = { > + SET_RUNTIME_PM_OPS(madera_runtime_suspend, > + madera_runtime_resume, > + NULL) > +}; There is a macro helper for this I believe. > +const struct of_device_id madera_of_match[] = { > + { .compatible = "cirrus,cs47l35", .data = (void *)CS47L35 }, > + { .compatible = "cirrus,cs47l85", .data = (void *)CS47L85 }, > + { .compatible = "cirrus,cs47l90", .data = (void *)CS47L90 }, > + { .compatible = "cirrus,cs47l91", .data = (void *)CS47L91 }, > + { .compatible = "cirrus,wm1840", .data = (void *)WM1840 }, > + {}, No comma. > +}; > + ret = devm_gpio_request_one(madera->dev, > + madera->pdata.reset, > + GPIOF_DIR_OUT | GPIOF_INIT_LOW, > + "madera reset"); > + if (!ret) > + madera->reset_gpio = gpio_to_desc(madera->pdata.reset); Why? What's wrong with descriptors? > + dev_set_drvdata(madera->dev, madera); ... > + if (dev_get_platdata(madera->dev)) What this dance for? > + ret = mfd_add_devices(madera->dev, PLATFORM_DEVID_NONE, > + mfd_devs, n_devs, > + NULL, 0, NULL); devm_? > + if (i2c->dev.of_node) { > + of_id = of_match_device(madera_of_match, &i2c->dev); > + if (of_id) > + type = (unsigned long)of_id->data; > + } else { > + type = id->driver_data; > + } > + if (spi->dev.of_node) { > + of_id = of_match_device(madera_of_match, &spi->dev); > + if (of_id) > + type = (unsigned long)of_id->data; There is a helper to get match data. > + } else { > + type = id->driver_data; > + } > +struct madera_irqchip_pdata; > +struct madera_codec_pdata; Why do you need platform data in new code? > + * @reset: GPIO controlling /RESET (0 = none) Shouldn't be descriptor?
On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > This adds support for the GPIOs on Cirrus Logic Madera class codecs. > Any pins not used for special functions (see the pinctrl driver) can be > used as general single-bit input or output lines. The number of available > GPIOs varies between codecs. > > Note that this is part of a composite MFD for these codecs and can only > be used with the corresponding MFD and other child drivers on those > silicon. The GPIO block on these codecs does not exist indepedently of > the rest of the MFD. > +struct madera_gpio { > + struct madera *madera; > + struct gpio_chip gpio_chip; > +}; Why do you need this? I suppose one embeds or refers to the other.
On 26/02/18 13:58, Fabio Estevam wrote: > On Mon, Feb 26, 2018 at 10:05 AM, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: > >> --- /dev/null >> +++ b/drivers/mfd/madera-core.c >> @@ -0,0 +1,602 @@ >> +// SPDX-License-Identifier: GPL-2.0 > > As you use SPDX line here.... > >> +/* >> + * Core MFD support for Cirrus Logic Madera codecs >> + * >> + * Copyright (C) 2015-2018 Cirrus Logic >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by the >> + * Free Software Foundation; version 2. > > You can remove this paragraph. > > Same applies to the other places of this series. > Our legal team says no, these lines should stay. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/02/18 14:11, Andy Shevchenko wrote: > On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: >> This adds the generic core support for Cirrus Logic "Madera" class codecs. >> These are complex audio codec SoCs with a variety of digital and analogue >> I/O, onboard audio processing and DSPs, and other features. >> >> These codecs are all based off a common set of hardware IP so can be >> supported by a core of common code (with a few minor device-to-device >> variations). > > >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by the >> + * Free Software Foundation; version 2. > > This is redundant. > Ditto my other reply. Our legal team want these lines. >> +static void madera_enable_hard_reset(struct madera *madera) >> +{ >> + if (madera->reset_gpio) > > if (!...) > return > Could do but why bother? For such a trivial function, in my opinion static void madera_enable_hard_reset(struct madera *madera) { if (madera->reset_gpio) gpiod_set_value_cansleep(madera->reset_gpio, 0); } is simpler and more readable than static void madera_enable_hard_reset(struct madera *madera) { if (!madera->reset_gpio) return; gpiod_set_value_cansleep(madera->reset_gpio, 0); } >> + gpiod_set_value_cansleep(madera->reset_gpio, 0); >> +} >> + >> +static void madera_disable_hard_reset(struct madera *madera) >> +{ >> + if (madera->reset_gpio) { > > Ditto. > As above, yes it would work the other way but I think for such a simple implementation the way I have written it is more readable. >> + gpiod_set_value_cansleep(madera->reset_gpio, 1); >> + usleep_range(1000, 2000); >> + } >> +} >> + > >> +#ifdef CONFIG_PM > > __maybe_unused > > >> +const struct dev_pm_ops madera_pm_ops = { >> + SET_RUNTIME_PM_OPS(madera_runtime_suspend, >> + madera_runtime_resume, >> + NULL) >> +}; > > There is a macro helper for this I believe. Not for a dev_pm_ops that only has runtime pm. If you're thinking of UNIVERSAL_DEV_PM_OPS that would set the same functions as handlers for system suspend, which we don't want to do for the reasons given in the comment describing UNIVERSAL_DEV_PM_OPS. > >> +const struct of_device_id madera_of_match[] = { >> + { .compatible = "cirrus,cs47l35", .data = (void *)CS47L35 }, >> + { .compatible = "cirrus,cs47l85", .data = (void *)CS47L85 }, >> + { .compatible = "cirrus,cs47l90", .data = (void *)CS47L90 }, >> + { .compatible = "cirrus,cs47l91", .data = (void *)CS47L91 }, >> + { .compatible = "cirrus,wm1840", .data = (void *)WM1840 }, > >> + {}, > > No comma. > Seems to be personal preference. Both ways are used in the kernel and we've always used this style so I'll leave it to Lee to decide. >> +}; > > >> + ret = devm_gpio_request_one(madera->dev, >> + madera->pdata.reset, >> + GPIOF_DIR_OUT | GPIOF_INIT_LOW, >> + "madera reset"); >> + if (!ret) >> + madera->reset_gpio = gpio_to_desc(madera->pdata.reset); > > Why? What's wrong with descriptors? > This is what I mean by code going stale when it's acked but then never gets merged. Some time ago there was a reason (which I forget). >> + dev_set_drvdata(madera->dev, madera); > ... >> + if (dev_get_platdata(madera->dev)) > > What this dance for? > Are you perhaps thinking the second line is dev_get_drvdata()? dev_get_platdata() gets a pointer to any pdata, so not related to dev_set_drvdata(). >> + ret = mfd_add_devices(madera->dev, PLATFORM_DEVID_NONE, >> + mfd_devs, n_devs, >> + NULL, 0, NULL); > > devm_? > I can try it and see. It's scary because we can depend on our children but maybe devm_mfd_add_devices() is safe. >> + if (i2c->dev.of_node) { >> + of_id = of_match_device(madera_of_match, &i2c->dev); >> + if (of_id) >> + type = (unsigned long)of_id->data; >> + } else { >> + type = id->driver_data; >> + } > >> + if (spi->dev.of_node) { >> + of_id = of_match_device(madera_of_match, &spi->dev); >> + if (of_id) >> + type = (unsigned long)of_id->data; > > There is a helper to get match data. > >> + } else { >> + type = id->driver_data; >> + } > >> +struct madera_irqchip_pdata; >> +struct madera_codec_pdata; > > > Why do you need platform data in new code? > Answered in a comment in another patch. We care about allowing people to use our chips with systems that don't use devicetree/acpi. There are also many out-of-tree systems. >> + * @reset: GPIO controlling /RESET (0 = none) > > Shouldn't be descriptor? > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 26, 2018 at 7:06 PM, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > On 26/02/18 14:11, Andy Shevchenko wrote: >> On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald >> <rf@opensource.cirrus.com> wrote: >>> +static void madera_enable_hard_reset(struct madera *madera) >>> +{ >>> + if (madera->reset_gpio) >> >> >> if (!...) >> return >> > > Could do but why bother? For such a trivial function, in my opinion > > static void madera_enable_hard_reset(struct madera *madera) > { > if (madera->reset_gpio) > gpiod_set_value_cansleep(madera->reset_gpio, 0); > } > > is simpler and more readable than > > static void madera_enable_hard_reset(struct madera *madera) > { > if (!madera->reset_gpio) > return; > > gpiod_set_value_cansleep(madera->reset_gpio, 0); > } The rationale is that if someone wants to add more code you will not need to take care of deeper indentation and potentially split lines. > >>> + gpiod_set_value_cansleep(madera->reset_gpio, 0); >>> +} >>> + >>> +static void madera_disable_hard_reset(struct madera *madera) >>> +{ >>> + if (madera->reset_gpio) { >> >> >> Ditto. >> > > As above, yes it would work the other way but I think for such a simple > implementation the way I have written it is more readable. I have different opinion, but yes. It's more matter of taste with rationale above (perhaps never happen to this code). >>> + gpiod_set_value_cansleep(madera->reset_gpio, 1); >>> + usleep_range(1000, 2000); >>> + } >>> +} >>> +const struct of_device_id madera_of_match[] = { >>> + { .compatible = "cirrus,cs47l35", .data = (void *)CS47L35 }, >>> + { .compatible = "cirrus,cs47l85", .data = (void *)CS47L85 }, >>> + { .compatible = "cirrus,cs47l90", .data = (void *)CS47L90 }, >>> + { .compatible = "cirrus,cs47l91", .data = (void *)CS47L91 }, >>> + { .compatible = "cirrus,wm1840", .data = (void *)WM1840 }, >> >> >>> + {}, >> >> >> No comma. >> > > Seems to be personal preference. Both ways are used in the kernel and > we've always used this style so I'll leave it to Lee to decide. This is not. The rationale is pretty obvious, terminator must terminate. With cheap price (no comma), we just prevent some potential weird cases (bad patch application for example, or not very careful contributor) where entry goes after. Compiler will fail. > >>> +}; >>> + ret = devm_gpio_request_one(madera->dev, >>> + madera->pdata.reset, >>> + GPIOF_DIR_OUT | >>> GPIOF_INIT_LOW, >>> + "madera reset"); >>> + if (!ret) >>> + madera->reset_gpio = >>> gpio_to_desc(madera->pdata.reset); >> >> >> Why? What's wrong with descriptors? > This is what I mean by code going stale when it's acked but then never > gets merged. Some time ago there was a reason (which I forget). So, can we switch to descriptors? >>> + dev_set_drvdata(madera->dev, madera); >>> + if (dev_get_platdata(madera->dev)) >> >> >> What this dance for? >> > > Are you perhaps thinking the second line is dev_get_drvdata()? > dev_get_platdata() gets a pointer to any pdata, so not related > to dev_set_drvdata(). Indeed. >>> + ret = mfd_add_devices(madera->dev, PLATFORM_DEVID_NONE, >>> + mfd_devs, n_devs, >>> + NULL, 0, NULL); >> >> >> devm_? >> > > I can try it and see. It's scary because we can depend on our > children but maybe devm_mfd_add_devices() is safe. It will fail in the same way. It does nothing more, than keeping a pointer to release function and its data. >>> +struct madera_irqchip_pdata; >>> +struct madera_codec_pdata; >> Why do you need platform data in new code? > Answered in a comment in another patch. We care about allowing people > to use our chips with systems that don't use devicetree/acpi. There > are also many out-of-tree systems. a) we don't care about out of tree much; b) there are other means to provide date w/o using platform data: - unified device property API (including built-in device properties) - bunch of lookup tables GPIO, regulator, PWM, etc - fwnode graph for more complex cases with device dependencies
On 26/02/18 14:16, Andy Shevchenko wrote: > On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: >> This adds support for the GPIOs on Cirrus Logic Madera class codecs. >> Any pins not used for special functions (see the pinctrl driver) can be >> used as general single-bit input or output lines. The number of available >> GPIOs varies between codecs. >> >> Note that this is part of a composite MFD for these codecs and can only >> be used with the corresponding MFD and other child drivers on those >> silicon. The GPIO block on these codecs does not exist indepedently of >> the rest of the MFD. > >> +struct madera_gpio { >> + struct madera *madera; >> + struct gpio_chip gpio_chip; >> +}; > > Why do you need this? I suppose one embeds or refers to the other. > The gpio_chip is storage for the struct gpio_chip that we're using. It allow us to create dynamically constructed struct gpio_chip based on the template in madera_gpio_chip. See the code in madera_gpio_probe. There are slight differences between the codecs (currently what we care about is that they have different numbers of gpios.) and as we still want to support pdata we also might have to change .base at runtime. It's possible to have multiple codecs of different type in one system so patching the static madera_gpio_chip is not viable, we would need different content for each codec. As almost all members are the same, what we do is use madera_gpio_chip as a template and copy it into the gpio_chip member of struct madera_gpio for this codec. That becomes the struct gpio_chip that we're actually using for this instance of this codec. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/02/18 17:19, Andy Shevchenko wrote: > On Mon, Feb 26, 2018 at 7:06 PM, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: >> On 26/02/18 14:11, Andy Shevchenko wrote: >>> On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald >>> <rf@opensource.cirrus.com> wrote: > >>>> +static void madera_enable_hard_reset(struct madera *madera) >>>> +{ >>>> + if (madera->reset_gpio) >>> >>> >>> if (!...) >>> return >>> >> >> Could do but why bother? For such a trivial function, in my opinion >> >> static void madera_enable_hard_reset(struct madera *madera) >> { >> if (madera->reset_gpio) >> gpiod_set_value_cansleep(madera->reset_gpio, 0); >> } >> >> is simpler and more readable than >> >> static void madera_enable_hard_reset(struct madera *madera) >> { >> if (!madera->reset_gpio) >> return; >> >> gpiod_set_value_cansleep(madera->reset_gpio, 0); >> } > > The rationale is that if someone wants to add more code you will not > need to take care of deeper indentation and potentially split lines. > Yes, true. It's probably unlikely here and I'm inclined to leave it as it is because Lee already acked it. >> >>>> + gpiod_set_value_cansleep(madera->reset_gpio, 0); >>>> +} >>>> + >>>> +static void madera_disable_hard_reset(struct madera *madera) >>>> +{ >>>> + if (madera->reset_gpio) { >>> >>> >>> Ditto. >>> >> >> As above, yes it would work the other way but I think for such a simple >> implementation the way I have written it is more readable. > > I have different opinion, but yes. It's more matter of taste with > rationale above (perhaps never happen to this code). > >>>> + gpiod_set_value_cansleep(madera->reset_gpio, 1); >>>> + usleep_range(1000, 2000); >>>> + } >>>> +} > >>>> +const struct of_device_id madera_of_match[] = { >>>> + { .compatible = "cirrus,cs47l35", .data = (void *)CS47L35 }, >>>> + { .compatible = "cirrus,cs47l85", .data = (void *)CS47L85 }, >>>> + { .compatible = "cirrus,cs47l90", .data = (void *)CS47L90 }, >>>> + { .compatible = "cirrus,cs47l91", .data = (void *)CS47L91 }, >>>> + { .compatible = "cirrus,wm1840", .data = (void *)WM1840 }, >>> >>> >>>> + {}, >>> >>> >>> No comma. >>> >> >> Seems to be personal preference. Both ways are used in the kernel and >> we've always used this style so I'll leave it to Lee to decide. > > This is not. > The rationale is pretty obvious, terminator must terminate. With cheap > price (no comma), we just prevent some potential weird cases (bad > patch application for example, or not very careful contributor) where > entry goes after. Compiler will fail. > Yes, ok. I see a lot of people don't do that (I searched). But if I do a new version of this patch I'll change it. >> >>>> +}; > >>>> + ret = devm_gpio_request_one(madera->dev, >>>> + madera->pdata.reset, >>>> + GPIOF_DIR_OUT | >>>> GPIOF_INIT_LOW, >>>> + "madera reset"); >>>> + if (!ret) >>>> + madera->reset_gpio = >>>> gpio_to_desc(madera->pdata.reset); >>> >>> >>> Why? What's wrong with descriptors? > >> This is what I mean by code going stale when it's acked but then never >> gets merged. Some time ago there was a reason (which I forget). > > So, can we switch to descriptors? > Yes, however as this patch has been in review for nearly 1 year now, and acked for several months, I'd really hoped we could get it merged now and update it later. >>>> + dev_set_drvdata(madera->dev, madera); >>>> + if (dev_get_platdata(madera->dev)) >>> >>> >>> What this dance for? >>> >> >> Are you perhaps thinking the second line is dev_get_drvdata()? >> dev_get_platdata() gets a pointer to any pdata, so not related >> to dev_set_drvdata(). > > Indeed. > >>>> + ret = mfd_add_devices(madera->dev, PLATFORM_DEVID_NONE, >>>> + mfd_devs, n_devs, >>>> + NULL, 0, NULL); >>> >>> >>> devm_? >>> >> >> I can try it and see. It's scary because we can depend on our >> children but maybe devm_mfd_add_devices() is safe. > > It will fail in the same way. It does nothing more, than keeping a > pointer to release function and its data. > >>>> +struct madera_irqchip_pdata; >>>> +struct madera_codec_pdata; > >>> Why do you need platform data in new code? > >> Answered in a comment in another patch. We care about allowing people >> to use our chips with systems that don't use devicetree/acpi. There >> are also many out-of-tree systems. > > a) we don't care about out of tree much; You might not, but as a commercial company we have to. > b) there are other means to provide date w/o using platform data: > - unified device property API (including built-in device properties) > - bunch of lookup tables GPIO, regulator, PWM, etc > - fwnode graph for more complex cases with device dependencies > Basically same answer as (a) -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/02/18 13:46, Andy Shevchenko wrote: > On Mon, Feb 26, 2018 at 3:05 PM, Richard Fitzgerald > <rf@opensource.cirrus.com> wrote: >> This patch adds a header file of register definitions for Cirrus >> Logic "Madera" class codecs. These codecs are all based off a common >> set of hardware IP so have a common register map (with a few minor >> device-to-device variations). >> >> The registers.h file is tool-generated directly from the hardware design >> but has been manually stripped down to reduce size (full register >> map is >44000 lines). All names are kept the same as datasheet names >> so that they can be cross-referenced between source and datasheet without >> confusion. >> >> The register map layout is kept fully-defined rather than factored into >> macros and/or block-indexing code. The major reasons for this are: >> >> - #1 is that it makes the source highly greppable, which is important. >> "What does the driver do with register bits XYZ" or "Where does it use >> register bits XYZ" are commonly types of questions. These can be quickly >> answered by a grep. Squashing definitions into generator macros or block- >> indexing code is a way of defeating grep. >> >> - most of the register definitions are used in tables, so a constant value >> is required. Using generator macros make the table definition clunky and >> obscure. >> >> - the code is clearer when it's there in the source exactly what register >> and field it is using >> >> - it is easier to diff the register map of a new (unsupported) codec against >> what is already supported and merge in differences >> >> - it makes the register map available in source for maintenance/debugging >> instead of having to refer back to the datasheet for a register map > > ... > >> +#define MADERA_OTP_HPDET_GRADIENT_1X_MASK 0x0000FF00 > >> +#define MADERA_OTP_HPDET_GRADIENT_0X_MASK 0x000000FF > > GENMASK() for all masks? > I'm an un-fan of GENMASK(). It seems to me to make it less readable. Unless it's important I'd rather leave the masks as they are because this is all auto-generated from the hardware design. Also keeping it in plain form like this makes it easier for people converting settings from the output of codec tuning tools. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 26, 2018 at 2:05 PM, Richard Fitzgerald <rf@opensource.cirrus.com> wrote: > These codecs have a variable number of I/O lines each of which > is individually selectable to a wide range of possible functions. > > The functionality is slightly different from the traditional muxed > GPIO since most of the functions can be mapped to any pin (and even > the same function to multiple pins). Most pins have a dedicated > "alternate" function that is only available on that pin. The > alternate functions are usually a group of signals, though it is > not always necessary to enable the full group, depending on the > alternate function and how it is to be used. The mapping between > alternate functions and GPIO pins varies between codecs depending > on the number of alternate functions and available pins. > > Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- > There are some minor changes since LinusW last reviewed this patch but as > they are trivial I have carried forward Linus's Reviewed-by: > - SPDX license headers > - can now build it as a module > - avoided a minor checkpatch warning about an unnecessary else {} in > madera_get_group_name() It's fine. Keep my Review tag. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html