* [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings @ 2019-02-24 8:27 Pankaj Bansal 2019-02-24 8:27 ` [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux Pankaj Bansal 2019-02-26 19:53 ` [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings Rob Herring 0 siblings, 2 replies; 11+ messages in thread From: Pankaj Bansal @ 2019-02-24 8:27 UTC (permalink / raw) To: Leo Li, Peter Rosin, Rob Herring, Frank Rowand; +Cc: Pankaj Bansal, devicetree This adds device tree binding documentation for generic register based multiplexer controlled by a bitfields in a parent device's register range. Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> --- Notes: V3: - Added the patch in series with the driver patch - Fixed the node value out of bitfield error - removed the "parent register r/w functions" line V2: - Removed syscon reference from txt file - Removed loading zeroes from hex numbers - Fixed the depth of dts nodes - fixed minor formatting errors .../devicetree/bindings/mux/reg-mux.txt | 83 ++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/Documentation/devicetree/bindings/mux/reg-mux.txt b/Documentation/devicetree/bindings/mux/reg-mux.txt new file mode 100644 index 000000000000..8bea6129c113 --- /dev/null +++ b/Documentation/devicetree/bindings/mux/reg-mux.txt @@ -0,0 +1,83 @@ +Generic register bitfield-based multiplexer controller bindings + +Define register bitfields to be used to control multiplexers. The parent +device tree node must be a device node to provide register r/w access. + +Required properties: +- compatible : "reg-mux" +- #mux-control-cells : <1> +- mux-reg-masks : an array of register offset and pre-shifted bitfield mask + pairs, each describing a single mux control. +* Standard mux-controller bindings as decribed in mux-controller.txt + +Optional properties: +- idle-states : if present, the state the muxes will have when idle. The + special state MUX_IDLE_AS_IS is the default. + +The multiplexer state of each multiplexer is defined as the value of the +bitfield described by the corresponding register offset and bitfield mask pair +in the mux-reg-masks array. + +Example: + +&i2c0 { + fpga@66 { // fpga connected to i2c + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c", + "simple-mfd"; + reg = <0x66>; + + mux: mux-controller { // Mux Producer + compatible = "reg-mux"; + #mux-control-cells = <1>; + mux-reg-masks = <0x54 0xf8>, /* 0: reg 0x54, bits 7:3 */ + <0x54 0x07>; /* 1: reg 0x54, bits 2:0 */ + }; + }; +}; + +mdio-mux-1 { // Mux consumer + compatible = "mdio-mux"; + mux-controls = <&mux 0>; + mdio-parent-bus = <&emdio1>; + #address-cells = <1>; + #size-cells = <0>; + + mdio@0 { + reg = <0x0>; + #address-cells = <1>; + #size-cells = <0>; + }; + + mdio@8 { + reg = <0x8>; + #address-cells = <1>; + #size-cells = <0>; + }; + + .. + .. +}; + +mdio-mux-2 { // Mux consumer + compatible = "mdio-mux"; + mux-controls = <&mux 1>; + mdio-parent-bus = <&emdio2>; + #address-cells = <1>; + #size-cells = <0>; + + mdio@0 { + reg = <0x0>; + #address-cells = <1>; + #size-cells = <0>; + }; + + mdio@1 { + reg = <0x1>; + #address-cells = <1>; + #size-cells = <0>; + }; + + .. + .. +}; + -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux 2019-02-24 8:27 [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings Pankaj Bansal @ 2019-02-24 8:27 ` Pankaj Bansal 2019-02-25 14:43 ` Peter Rosin 2019-02-26 19:53 ` [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings Rob Herring 1 sibling, 1 reply; 11+ messages in thread From: Pankaj Bansal @ 2019-02-24 8:27 UTC (permalink / raw) To: Leo Li, Peter Rosin; +Cc: Pankaj Bansal, linux-kernel Generic register bitfield-based multiplexer that controls the multiplexer producer defined under a parent node. The driver corresponding to parent node provides register read/write capabilities. Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> --- Notes: V3: - Added the patch in series with device tree binding patch - Added the NULL return handling for regmap V2: - removed seperate driver regmap.c and added the regmap function in mmio.c based on compatible field, the syscon or regmap function would be called - Modified the KConfig as per Peter's comments drivers/mux/Kconfig | 12 ++++++------ drivers/mux/mmio.c | 10 +++++++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index 7659d6c5f718..e5c571fd232c 100644 --- a/drivers/mux/Kconfig +++ b/drivers/mux/Kconfig @@ -46,14 +46,14 @@ config MUX_GPIO be called mux-gpio. config MUX_MMIO - tristate "MMIO register bitfield-controlled Multiplexer" - depends on (OF && MFD_SYSCON) || COMPILE_TEST + tristate "MMIO/Regmap register bitfield-controlled Multiplexer" + depends on OF || COMPILE_TEST help - MMIO register bitfield-controlled Multiplexer controller. + MMIO/Regmap register bitfield-controlled Multiplexer controller. - The driver builds multiplexer controllers for bitfields in a syscon - register. For N bit wide bitfields, there will be 2^N possible - multiplexer states. + The driver builds multiplexer controllers for bitfields in either + a syscon register or a driver regmap register. For N bit wide + bitfields, there will be 2^N possible multiplexer states. To compile the driver as a module, choose M here: the module will be called mux-mmio. diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c index 935ac44aa209..cc02155e4644 100644 --- a/drivers/mux/mmio.c +++ b/drivers/mux/mmio.c @@ -28,6 +28,7 @@ static const struct mux_control_ops mux_mmio_ops = { static const struct of_device_id mux_mmio_dt_ids[] = { { .compatible = "mmio-mux", }, + { .compatible = "reg-mux", }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids); @@ -43,9 +44,12 @@ static int mux_mmio_probe(struct platform_device *pdev) int ret; int i; - regmap = syscon_node_to_regmap(np->parent); - if (IS_ERR(regmap)) { - ret = PTR_ERR(regmap); + if (of_device_is_compatible(np, "mmio-mux")) + regmap = syscon_node_to_regmap(np->parent); + else + regmap = dev_get_regmap(dev->parent, NULL); + if (IS_ERR_OR_NULL(regmap)) { + ret = PTR_ERR_OR_ZERO(regmap) ? PTR_ERR(regmap) : -ENODEV; dev_err(dev, "failed to get regmap: %d\n", ret); return ret; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux 2019-02-24 8:27 ` [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux Pankaj Bansal @ 2019-02-25 14:43 ` Peter Rosin 2019-02-26 6:08 ` Pankaj Bansal 0 siblings, 1 reply; 11+ messages in thread From: Peter Rosin @ 2019-02-25 14:43 UTC (permalink / raw) To: Pankaj Bansal, Leo Li; +Cc: linux-kernel On 2019-02-24 09:27, Pankaj Bansal wrote: > Generic register bitfield-based multiplexer that controls the multiplexer > producer defined under a parent node. > The driver corresponding to parent node provides register read/write > capabilities. > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> > --- > > Notes: > V3: > - Added the patch in series with device tree binding patch > - Added the NULL return handling for regmap > V2: > - removed seperate driver regmap.c and added the regmap function in mmio.c > based on compatible field, the syscon or regmap function would be called > - Modified the KConfig as per Peter's comments > > drivers/mux/Kconfig | 12 ++++++------ > drivers/mux/mmio.c | 10 +++++++--- > 2 files changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig > index 7659d6c5f718..e5c571fd232c 100644 > --- a/drivers/mux/Kconfig > +++ b/drivers/mux/Kconfig > @@ -46,14 +46,14 @@ config MUX_GPIO > be called mux-gpio. > > config MUX_MMIO > - tristate "MMIO register bitfield-controlled Multiplexer" > - depends on (OF && MFD_SYSCON) || COMPILE_TEST > + tristate "MMIO/Regmap register bitfield-controlled Multiplexer" > + depends on OF || COMPILE_TEST > help > - MMIO register bitfield-controlled Multiplexer controller. > + MMIO/Regmap register bitfield-controlled Multiplexer controller. > > - The driver builds multiplexer controllers for bitfields in a syscon > - register. For N bit wide bitfields, there will be 2^N possible > - multiplexer states. > + The driver builds multiplexer controllers for bitfields in either > + a syscon register or a driver regmap register. For N bit wide > + bitfields, there will be 2^N possible multiplexer states. > > To compile the driver as a module, choose M here: the module will > be called mux-mmio. > diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c > index 935ac44aa209..cc02155e4644 100644 > --- a/drivers/mux/mmio.c > +++ b/drivers/mux/mmio.c > @@ -28,6 +28,7 @@ static const struct mux_control_ops mux_mmio_ops = { > > static const struct of_device_id mux_mmio_dt_ids[] = { > { .compatible = "mmio-mux", }, > + { .compatible = "reg-mux", }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids); > @@ -43,9 +44,12 @@ static int mux_mmio_probe(struct platform_device *pdev) > int ret; > int i; > > - regmap = syscon_node_to_regmap(np->parent); > - if (IS_ERR(regmap)) { > - ret = PTR_ERR(regmap); > + if (of_device_is_compatible(np, "mmio-mux")) > + regmap = syscon_node_to_regmap(np->parent); > + else > + regmap = dev_get_regmap(dev->parent, NULL); > + if (IS_ERR_OR_NULL(regmap)) { > + ret = PTR_ERR_OR_ZERO(regmap) ? PTR_ERR(regmap) : -ENODEV; The above is not correct, this should be better (untested): ret = PTR_ERR(regmap) ?: -ENODEV; Cheers, Peter > dev_err(dev, "failed to get regmap: %d\n", ret); > return ret; > } > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux 2019-02-25 14:43 ` Peter Rosin @ 2019-02-26 6:08 ` Pankaj Bansal 2019-02-26 8:20 ` Peter Rosin 0 siblings, 1 reply; 11+ messages in thread From: Pankaj Bansal @ 2019-02-26 6:08 UTC (permalink / raw) To: Peter Rosin, Leo Li; +Cc: linux-kernel Hi Peter, > -----Original Message----- > From: Peter Rosin [mailto:peda@axentia.se] > Sent: Monday, 25 February, 2019 08:14 PM > To: Pankaj Bansal <pankaj.bansal@nxp.com>; Leo Li <leoyang.li@nxp.com> > Cc: linux-kernel@vger.kernel.org > Subject: Re: [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based > multiplexer in mmio-mux > > On 2019-02-24 09:27, Pankaj Bansal wrote: > > Generic register bitfield-based multiplexer that controls the > > multiplexer producer defined under a parent node. > > The driver corresponding to parent node provides register read/write > > capabilities. > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> > > --- > > > > Notes: > > V3: > > - Added the patch in series with device tree binding patch > > - Added the NULL return handling for regmap > > V2: > > - removed seperate driver regmap.c and added the regmap function in > mmio.c > > based on compatible field, the syscon or regmap function would be called > > - Modified the KConfig as per Peter's comments > > > > drivers/mux/Kconfig | 12 ++++++------ drivers/mux/mmio.c | 10 > > +++++++--- > > 2 files changed, 13 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/mux/Kconfig b/drivers/mux/Kconfig index > > 7659d6c5f718..e5c571fd232c 100644 > > --- a/drivers/mux/Kconfig > > +++ b/drivers/mux/Kconfig > > @@ -46,14 +46,14 @@ config MUX_GPIO > > be called mux-gpio. > > > > config MUX_MMIO > > - tristate "MMIO register bitfield-controlled Multiplexer" > > - depends on (OF && MFD_SYSCON) || COMPILE_TEST > > + tristate "MMIO/Regmap register bitfield-controlled Multiplexer" > > + depends on OF || COMPILE_TEST > > help > > - MMIO register bitfield-controlled Multiplexer controller. > > + MMIO/Regmap register bitfield-controlled Multiplexer controller. > > > > - The driver builds multiplexer controllers for bitfields in a syscon > > - register. For N bit wide bitfields, there will be 2^N possible > > - multiplexer states. > > + The driver builds multiplexer controllers for bitfields in either > > + a syscon register or a driver regmap register. For N bit wide > > + bitfields, there will be 2^N possible multiplexer states. > > > > To compile the driver as a module, choose M here: the module will > > be called mux-mmio. > > diff --git a/drivers/mux/mmio.c b/drivers/mux/mmio.c index > > 935ac44aa209..cc02155e4644 100644 > > --- a/drivers/mux/mmio.c > > +++ b/drivers/mux/mmio.c > > @@ -28,6 +28,7 @@ static const struct mux_control_ops mux_mmio_ops = { > > > > static const struct of_device_id mux_mmio_dt_ids[] = { > > { .compatible = "mmio-mux", }, > > + { .compatible = "reg-mux", }, > > { /* sentinel */ } > > }; > > MODULE_DEVICE_TABLE(of, mux_mmio_dt_ids); @@ -43,9 +44,12 @@ > static > > int mux_mmio_probe(struct platform_device *pdev) > > int ret; > > int i; > > > > - regmap = syscon_node_to_regmap(np->parent); > > - if (IS_ERR(regmap)) { > > - ret = PTR_ERR(regmap); > > + if (of_device_is_compatible(np, "mmio-mux")) > > + regmap = syscon_node_to_regmap(np->parent); > > + else > > + regmap = dev_get_regmap(dev->parent, NULL); > > + if (IS_ERR_OR_NULL(regmap)) { > > + ret = PTR_ERR_OR_ZERO(regmap) ? PTR_ERR(regmap) : - > ENODEV; > > The above is not correct, this should be better (untested): > > ret = PTR_ERR(regmap) ?: -ENODEV; Omitting the second operand in ternary operator is not standard. https://stackoverflow.com/questions/34559705/ternary-operator-without-the-middle-expression Although, it *has been* used in kernel in many places https://livegrep.com/search/linux?q=file%3A%5C.c%24%20%5C%3F%5C%3A&fold_case=auto®ex=true&context=true > > Cheers, > Peter > > > dev_err(dev, "failed to get regmap: %d\n", ret); > > return ret; > > } > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux 2019-02-26 6:08 ` Pankaj Bansal @ 2019-02-26 8:20 ` Peter Rosin 0 siblings, 0 replies; 11+ messages in thread From: Peter Rosin @ 2019-02-26 8:20 UTC (permalink / raw) To: Pankaj Bansal, Leo Li; +Cc: linux-kernel On 2019-02-26 07:08, Pankaj Bansal wrote: >>> + if (IS_ERR_OR_NULL(regmap)) { >>> + ret = PTR_ERR_OR_ZERO(regmap) ? PTR_ERR(regmap) : - >> ENODEV; >> >> The above is not correct, this should be better (untested): >> >> ret = PTR_ERR(regmap) ?: -ENODEV; > > Omitting the second operand in ternary operator is not standard. https://stackoverflow.com/questions/34559705/ternary-operator-without-the-middle-expression Irrelevant, a great many non-standard features are used in the kernel. > Although, it *has been* used in kernel in many places > https://livegrep.com/search/linux?q=file%3A%5C.c%24%20%5C%3F%5C%3A&fold_case=auto®ex=true&context=true Yes, a local grep suggests that there are plenty of instances in the kernel. In fact, there are multiple instances even in the kernel/ directory so it's not some fringe thing. My conclusion is that it makes for readable code and is perfectly safe to use in the kernel, which apparently does not care about compilers that do not understand the construct. $ git grep '?:' *.[ch] | wc 773 4830 54975 I you still don't buy that and don't want to sign-off on it, then write ret = IS_ERR(regmap) ? PTR_ERR(regmap) : -ENODEV; But that's just wordy and less readable, methinks. However, the best thing to do, according to your maintainer, is still the original suggestion to fixup the NULL as early as possible with regmap = dev_get_regmap(dev->parent, NULL) ?: ERR_PTR(-ENODEV); And leave the error handling block alone. Can you please just do that and be done with it? Cheers, Peter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings 2019-02-24 8:27 [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings Pankaj Bansal 2019-02-24 8:27 ` [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux Pankaj Bansal @ 2019-02-26 19:53 ` Rob Herring 2019-02-26 20:28 ` Li Yang 1 sibling, 1 reply; 11+ messages in thread From: Rob Herring @ 2019-02-26 19:53 UTC (permalink / raw) To: Pankaj Bansal; +Cc: Leo Li, Peter Rosin, Frank Rowand, devicetree On Sun, Feb 24, 2019 at 08:27:23AM +0000, Pankaj Bansal wrote: > This adds device tree binding documentation for generic register based > multiplexer controlled by a bitfields in a parent device's register range. > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> > --- > > Notes: > V3: > - Added the patch in series with the driver patch > - Fixed the node value out of bitfield error > - removed the "parent register r/w functions" line > V2: > - Removed syscon reference from txt file > - Removed loading zeroes from hex numbers > - Fixed the depth of dts nodes > - fixed minor formatting errors > > .../devicetree/bindings/mux/reg-mux.txt | 83 ++++++++++++++++++ > 1 file changed, 83 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mux/reg-mux.txt b/Documentation/devicetree/bindings/mux/reg-mux.txt > new file mode 100644 > index 000000000000..8bea6129c113 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mux/reg-mux.txt > @@ -0,0 +1,83 @@ > +Generic register bitfield-based multiplexer controller bindings > + > +Define register bitfields to be used to control multiplexers. The parent > +device tree node must be a device node to provide register r/w access. We generally avoid register bit level bindings like this... What happens when I need 8 or 16-bit accesses. Or some quirky encoding of the bits. Or non-contiguous bit fields... It's an endless extending of the binding to try to handle different cases. > + > +Required properties: > +- compatible : "reg-mux" > +- #mux-control-cells : <1> > +- mux-reg-masks : an array of register offset and pre-shifted bitfield mask > + pairs, each describing a single mux control. > +* Standard mux-controller bindings as decribed in mux-controller.txt > + > +Optional properties: > +- idle-states : if present, the state the muxes will have when idle. The > + special state MUX_IDLE_AS_IS is the default. > + > +The multiplexer state of each multiplexer is defined as the value of the > +bitfield described by the corresponding register offset and bitfield mask pair > +in the mux-reg-masks array. > + > +Example: > + > +&i2c0 { > + fpga@66 { // fpga connected to i2c > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c", > + "simple-mfd"; > + reg = <0x66>; > + > + mux: mux-controller { // Mux Producer > + compatible = "reg-mux"; > + #mux-control-cells = <1>; > + mux-reg-masks = <0x54 0xf8>, /* 0: reg 0x54, bits 7:3 */ > + <0x54 0x07>; /* 1: reg 0x54, bits 2:0 */ You can accomplish the same thing by moving these 2 properties to the parent. The parent driver can register a generic mux if that's appropriate. Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings 2019-02-26 19:53 ` [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings Rob Herring @ 2019-02-26 20:28 ` Li Yang 2019-02-26 22:00 ` Rob Herring 0 siblings, 1 reply; 11+ messages in thread From: Li Yang @ 2019-02-26 20:28 UTC (permalink / raw) To: Rob Herring; +Cc: Pankaj Bansal, Peter Rosin, Frank Rowand, devicetree On Tue, Feb 26, 2019 at 1:54 PM Rob Herring <robh@kernel.org> wrote: > > On Sun, Feb 24, 2019 at 08:27:23AM +0000, Pankaj Bansal wrote: > > This adds device tree binding documentation for generic register based > > multiplexer controlled by a bitfields in a parent device's register range. > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> > > --- > > > > Notes: > > V3: > > - Added the patch in series with the driver patch > > - Fixed the node value out of bitfield error > > - removed the "parent register r/w functions" line > > V2: > > - Removed syscon reference from txt file > > - Removed loading zeroes from hex numbers > > - Fixed the depth of dts nodes > > - fixed minor formatting errors > > > > .../devicetree/bindings/mux/reg-mux.txt | 83 ++++++++++++++++++ > > 1 file changed, 83 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mux/reg-mux.txt b/Documentation/devicetree/bindings/mux/reg-mux.txt > > new file mode 100644 > > index 000000000000..8bea6129c113 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mux/reg-mux.txt > > @@ -0,0 +1,83 @@ > > +Generic register bitfield-based multiplexer controller bindings > > + > > +Define register bitfields to be used to control multiplexers. The parent > > +device tree node must be a device node to provide register r/w access. > > We generally avoid register bit level bindings like this... > > What happens when I need 8 or 16-bit accesses. Or some quirky encoding > of the bits. Or non-contiguous bit fields... It's an endless extending > of the binding to try to handle different cases. I think the intention here is to mimic the existing mmio-mux binding (Documentation/devicetree/bindings/mux/mmio-mux.txt) so that the existing mmio-mux driver can be extended to cover both MMIO and non-MMIO register based muxes. The bit-width of the register access will be taken care of by the regmap framework. It is probably true that this can not cover all the register based mux device if they do have quircky register layout, but I think it should be good enough to cover majority of the cases. If there are devices that cannot be covered by the generic binding and driver, they could create their own bindings and drivers. > > > + > > +Required properties: > > +- compatible : "reg-mux" > > +- #mux-control-cells : <1> > > +- mux-reg-masks : an array of register offset and pre-shifted bitfield mask > > + pairs, each describing a single mux control. > > +* Standard mux-controller bindings as decribed in mux-controller.txt > > + > > +Optional properties: > > +- idle-states : if present, the state the muxes will have when idle. The > > + special state MUX_IDLE_AS_IS is the default. > > + > > +The multiplexer state of each multiplexer is defined as the value of the > > +bitfield described by the corresponding register offset and bitfield mask pair > > +in the mux-reg-masks array. > > + > > +Example: > > + > > +&i2c0 { > > + fpga@66 { // fpga connected to i2c > > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c", > > + "simple-mfd"; > > + reg = <0x66>; > > + > > + mux: mux-controller { // Mux Producer > > + compatible = "reg-mux"; > > > + #mux-control-cells = <1>; > > + mux-reg-masks = <0x54 0xf8>, /* 0: reg 0x54, bits 7:3 */ > > + <0x54 0x07>; /* 1: reg 0x54, bits 2:0 */ > > You can accomplish the same thing by moving these 2 properties to the > parent. The parent driver can register a generic mux if that's > appropriate. Again, the intention is to reuse the generic mux binding and driver. Regards, Leo ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings 2019-02-26 20:28 ` Li Yang @ 2019-02-26 22:00 ` Rob Herring 2019-02-27 8:30 ` Pankaj Bansal 0 siblings, 1 reply; 11+ messages in thread From: Rob Herring @ 2019-02-26 22:00 UTC (permalink / raw) To: Li Yang; +Cc: Pankaj Bansal, Peter Rosin, Frank Rowand, devicetree On Tue, Feb 26, 2019 at 02:28:12PM -0600, Li Yang wrote: > On Tue, Feb 26, 2019 at 1:54 PM Rob Herring <robh@kernel.org> wrote: > > > > On Sun, Feb 24, 2019 at 08:27:23AM +0000, Pankaj Bansal wrote: > > > This adds device tree binding documentation for generic register based > > > multiplexer controlled by a bitfields in a parent device's register range. > > > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> > > > --- > > > > > > Notes: > > > V3: > > > - Added the patch in series with the driver patch > > > - Fixed the node value out of bitfield error > > > - removed the "parent register r/w functions" line > > > V2: > > > - Removed syscon reference from txt file > > > - Removed loading zeroes from hex numbers > > > - Fixed the depth of dts nodes > > > - fixed minor formatting errors > > > > > > .../devicetree/bindings/mux/reg-mux.txt | 83 ++++++++++++++++++ > > > 1 file changed, 83 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/mux/reg-mux.txt b/Documentation/devicetree/bindings/mux/reg-mux.txt > > > new file mode 100644 > > > index 000000000000..8bea6129c113 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/mux/reg-mux.txt > > > @@ -0,0 +1,83 @@ > > > +Generic register bitfield-based multiplexer controller bindings > > > + > > > +Define register bitfields to be used to control multiplexers. The parent > > > +device tree node must be a device node to provide register r/w access. > > > > We generally avoid register bit level bindings like this... > > > > What happens when I need 8 or 16-bit accesses. Or some quirky encoding > > of the bits. Or non-contiguous bit fields... It's an endless extending > > of the binding to try to handle different cases. > > I think the intention here is to mimic the existing mmio-mux binding > (Documentation/devicetree/bindings/mux/mmio-mux.txt) so that the > existing mmio-mux driver can be extended to cover both MMIO and > non-MMIO register based muxes. The bit-width of the register access > will be taken care of by the regmap framework. It is probably true > that this can not cover all the register based mux device if they do > have quircky register layout, but I think it should be good enough to > cover majority of the cases. If there are devices that cannot be > covered by the generic binding and driver, they could create their own > bindings and drivers. Other than the somewhat misleading name, why not just reuse the existing binding? Worst case, just add another compatible to it and renane the file. That would have saved me reviewing the whole thing again... But really, you should just be able to use what is there and determine the type of access from the parent node (or I suppose regmap does that for you). Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings 2019-02-26 22:00 ` Rob Herring @ 2019-02-27 8:30 ` Pankaj Bansal 2019-02-27 8:42 ` Peter Rosin 0 siblings, 1 reply; 11+ messages in thread From: Pankaj Bansal @ 2019-02-27 8:30 UTC (permalink / raw) To: 'Rob Herring', Leo Li, Peter Rosin; +Cc: Frank Rowand, devicetree Hi Rob, > -----Original Message----- > From: Rob Herring [mailto:robh@kernel.org] > Sent: Wednesday, 27 February, 2019 03:31 AM > To: Leo Li <leoyang.li@nxp.com> > Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Peter Rosin <peda@axentia.se>; > Frank Rowand <frowand.list@gmail.com>; devicetree@vger.kernel.org > Subject: Re: [PATCH v3 1/2] dt-bindings: add register based devices' mux > controller DT bindings > > On Tue, Feb 26, 2019 at 02:28:12PM -0600, Li Yang wrote: > > On Tue, Feb 26, 2019 at 1:54 PM Rob Herring <robh@kernel.org> wrote: > > > > > > On Sun, Feb 24, 2019 at 08:27:23AM +0000, Pankaj Bansal wrote: > > > > This adds device tree binding documentation for generic register > > > > based multiplexer controlled by a bitfields in a parent device's register > range. > > > > > > > > Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> > > > > --- > > > > > > > > Notes: > > > > V3: > > > > - Added the patch in series with the driver patch > > > > - Fixed the node value out of bitfield error > > > > - removed the "parent register r/w functions" line > > > > V2: > > > > - Removed syscon reference from txt file > > > > - Removed loading zeroes from hex numbers > > > > - Fixed the depth of dts nodes > > > > - fixed minor formatting errors > > > > > > > > .../devicetree/bindings/mux/reg-mux.txt | 83 ++++++++++++++++++ > > > > 1 file changed, 83 insertions(+) > > > > > > > > diff --git a/Documentation/devicetree/bindings/mux/reg-mux.txt > > > > b/Documentation/devicetree/bindings/mux/reg-mux.txt > > > > new file mode 100644 > > > > index 000000000000..8bea6129c113 > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/mux/reg-mux.txt > > > > @@ -0,0 +1,83 @@ > > > > +Generic register bitfield-based multiplexer controller bindings > > > > + > > > > +Define register bitfields to be used to control multiplexers. The > > > > +parent device tree node must be a device node to provide register r/w > access. > > > > > > We generally avoid register bit level bindings like this... > > > > > > What happens when I need 8 or 16-bit accesses. Or some quirky > > > encoding of the bits. Or non-contiguous bit fields... It's an > > > endless extending of the binding to try to handle different cases. > > > > I think the intention here is to mimic the existing mmio-mux binding > > (Documentation/devicetree/bindings/mux/mmio-mux.txt) so that the > > existing mmio-mux driver can be extended to cover both MMIO and > > non-MMIO register based muxes. The bit-width of the register access > > will be taken care of by the regmap framework. It is probably true > > that this can not cover all the register based mux device if they do > > have quircky register layout, but I think it should be good enough to > > cover majority of the cases. If there are devices that cannot be > > covered by the generic binding and driver, they could create their own > > bindings and drivers. > > Other than the somewhat misleading name, why not just reuse the existing > binding? Worst case, just add another compatible to it and renane the file. That > would have saved me reviewing the whole thing again... > I guess I can rename mmio-mux to reg-mux and add the bindings in it. Because really, the MMIO mux is a special case of register based mux, where the register access is memory mapped. @Peter Rosin : Are you okay with this? > But really, you should just be able to use what is there and determine the type of > access from the parent node (or I suppose regmap does that for you). The intention is to have an indication that the parent devices produces one (or many) mux Producers, which are controlled by parent devices' registers. Which is why I chose compatible "reg-mux". The parent device's register can be written in any way (MMIO / SPI / I2C) etc. > > Rob ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings 2019-02-27 8:30 ` Pankaj Bansal @ 2019-02-27 8:42 ` Peter Rosin 2019-02-27 8:46 ` Peter Rosin 0 siblings, 1 reply; 11+ messages in thread From: Peter Rosin @ 2019-02-27 8:42 UTC (permalink / raw) To: Pankaj Bansal, 'Rob Herring', Leo Li; +Cc: Frank Rowand, devicetree On 2019-02-27 09:30, Pankaj Bansal wrote: > Hi Rob, > >> -----Original Message----- >> From: Rob Herring [mailto:robh@kernel.org] >> Sent: Wednesday, 27 February, 2019 03:31 AM >> To: Leo Li <leoyang.li@nxp.com> >> Cc: Pankaj Bansal <pankaj.bansal@nxp.com>; Peter Rosin <peda@axentia.se>; >> Frank Rowand <frowand.list@gmail.com>; devicetree@vger.kernel.org >> Subject: Re: [PATCH v3 1/2] dt-bindings: add register based devices' mux >> controller DT bindings >> >> On Tue, Feb 26, 2019 at 02:28:12PM -0600, Li Yang wrote: >>> On Tue, Feb 26, 2019 at 1:54 PM Rob Herring <robh@kernel.org> wrote: >>>> >>>> On Sun, Feb 24, 2019 at 08:27:23AM +0000, Pankaj Bansal wrote: >>>>> This adds device tree binding documentation for generic register >>>>> based multiplexer controlled by a bitfields in a parent device's register >> range. >>>>> >>>>> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com> >>>>> --- >>>>> >>>>> Notes: >>>>> V3: >>>>> - Added the patch in series with the driver patch >>>>> - Fixed the node value out of bitfield error >>>>> - removed the "parent register r/w functions" line >>>>> V2: >>>>> - Removed syscon reference from txt file >>>>> - Removed loading zeroes from hex numbers >>>>> - Fixed the depth of dts nodes >>>>> - fixed minor formatting errors >>>>> >>>>> .../devicetree/bindings/mux/reg-mux.txt | 83 ++++++++++++++++++ >>>>> 1 file changed, 83 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/mux/reg-mux.txt >>>>> b/Documentation/devicetree/bindings/mux/reg-mux.txt >>>>> new file mode 100644 >>>>> index 000000000000..8bea6129c113 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/mux/reg-mux.txt >>>>> @@ -0,0 +1,83 @@ >>>>> +Generic register bitfield-based multiplexer controller bindings >>>>> + >>>>> +Define register bitfields to be used to control multiplexers. The >>>>> +parent device tree node must be a device node to provide register r/w >> access. >>>> >>>> We generally avoid register bit level bindings like this... >>>> >>>> What happens when I need 8 or 16-bit accesses. Or some quirky >>>> encoding of the bits. Or non-contiguous bit fields... It's an >>>> endless extending of the binding to try to handle different cases. >>> >>> I think the intention here is to mimic the existing mmio-mux binding >>> (Documentation/devicetree/bindings/mux/mmio-mux.txt) so that the >>> existing mmio-mux driver can be extended to cover both MMIO and >>> non-MMIO register based muxes. The bit-width of the register access >>> will be taken care of by the regmap framework. It is probably true >>> that this can not cover all the register based mux device if they do >>> have quircky register layout, but I think it should be good enough to >>> cover majority of the cases. If there are devices that cannot be >>> covered by the generic binding and driver, they could create their own >>> bindings and drivers. >> >> Other than the somewhat misleading name, why not just reuse the existing >> binding? Worst case, just add another compatible to it and renane the file. That >> would have saved me reviewing the whole thing again... >> > > I guess I can rename mmio-mux to reg-mux and add the bindings in it. Because really, the > MMIO mux is a special case of register based mux, where the register access is memory mapped. > @Peter Rosin : Are you okay with this? It's so obvious now that Rob points it out. Sorry for not realizing this all by myself when I reviewed the initial revisions. I made the connection for the driver itself, but not so for the binding. How silly, and sorry again for the trouble. So, please rename the file with the binding, and hack the text to cover both compatibles (mmio-mux and reg-mux), because I think it would be very confusing to reuse the mmio-mux compatible for a "reg-mux" that is not mmio based. >> But really, you should just be able to use what is there and determine the type of >> access from the parent node (or I suppose regmap does that for you). > > The intention is to have an indication that the parent devices produces one (or many) mux > Producers, which are controlled by parent devices' registers. > Which is why I chose compatible "reg-mux". This is not what Rob is talking about, he is talking about /not/ adding a new compatible at all, and have the driver simply try both of syscon_node_to_regmap(np->parent) and dev_get_regmap(dev->parent, NULL) (in some other) and go with anything that is returned. However, as I said above, that leads to confusing naming in the dts, and adding a new compatible feels like it's worth it. Cheers, Peter > The parent device's register can be written in any way (MMIO / SPI / I2C) etc. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings 2019-02-27 8:42 ` Peter Rosin @ 2019-02-27 8:46 ` Peter Rosin 0 siblings, 0 replies; 11+ messages in thread From: Peter Rosin @ 2019-02-27 8:46 UTC (permalink / raw) To: Pankaj Bansal, 'Rob Herring', Leo Li; +Cc: Frank Rowand, devicetree On 2019-02-27 09:42, Peter Rosin wrote: > This is not what Rob is talking about, he is talking about /not/ adding > a new compatible at all, and have the driver simply try both of > syscon_node_to_regmap(np->parent) and dev_get_regmap(dev->parent, NULL) > (in some other) and go with anything that is returned. "(in some order)", sigh... Cheers, Peter ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-02-27 8:46 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-24 8:27 [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings Pankaj Bansal 2019-02-24 8:27 ` [PATCH v3 2/2] drivers: mux: Add Generic regmap bitfield-based multiplexer in mmio-mux Pankaj Bansal 2019-02-25 14:43 ` Peter Rosin 2019-02-26 6:08 ` Pankaj Bansal 2019-02-26 8:20 ` Peter Rosin 2019-02-26 19:53 ` [PATCH v3 1/2] dt-bindings: add register based devices' mux controller DT bindings Rob Herring 2019-02-26 20:28 ` Li Yang 2019-02-26 22:00 ` Rob Herring 2019-02-27 8:30 ` Pankaj Bansal 2019-02-27 8:42 ` Peter Rosin 2019-02-27 8:46 ` Peter Rosin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.