Hi Greg,
These have been in -next since around x-mas, and hopefully it's not too
late to push them for the upcoming window.
Patch 5/6 fails check-patch due to lacking a message body, because I
forgot to actually check that and now I can't find it in me to re-spin
for that seemingly insignificant issue. If it really is a no-no, then
please just drop that patch.
Also, I really would have liked to squash patch 6/6 into patch 3/6, but
I don't know how to retain credits properly if I do that. I did remove
the Fixes tag from 6/6, since the commit hash will be trash by the time
you make your commit. It was:
Fixes: xxxxxxxxxxxx ("mux: Add support for reading mux state from consumer DT node")
(In case you can find it in you to add it back with a proper hash.)
Anyway, the big change here is the new support for pointing at a
specific mux state from a device tree node.
Cheers,
Peter
Aswath Govindraju (3):
dt-bindings: ti-serdes-mux: Add defines for J721S2 SoC
dt-bindings: mux: Document mux-states property
mux: Add support for reading mux state from consumer DT node
Peter Rosin (2):
mux: add missing mux_state_get
mux: fix grammar, missing "is".
Yang Li (1):
mux: Fix struct mux_state kernel-doc comment
.../devicetree/bindings/mux/gpio-mux.yaml | 11 +-
.../devicetree/bindings/mux/mux-consumer.yaml | 21 ++
.../devicetree/bindings/mux/mux-controller.yaml | 26 ++-
Documentation/driver-api/driver-model/devres.rst | 1 +
drivers/mux/core.c | 241 +++++++++++++++++++--
include/dt-bindings/mux/ti-serdes.h | 22 ++
include/linux/mux/consumer.h | 20 ++
7 files changed, 317 insertions(+), 25 deletions(-)
From: Aswath Govindraju <[email protected]>
There are 4 lanes in the single instance of J721S2 SERDES. Each SERDES
lane mux can select upto 4 different IPs. Define all the possible
functions.
Signed-off-by: Aswath Govindraju <[email protected]>
Acked-by: Rob Herring <[email protected]>
Signed-off-by: Peter Rosin <[email protected]>
---
include/dt-bindings/mux/ti-serdes.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/include/dt-bindings/mux/ti-serdes.h b/include/dt-bindings/mux/ti-serdes.h
index d417b9268b16..d3116c52ab72 100644
--- a/include/dt-bindings/mux/ti-serdes.h
+++ b/include/dt-bindings/mux/ti-serdes.h
@@ -95,4 +95,26 @@
#define AM64_SERDES0_LANE0_PCIE0 0x0
#define AM64_SERDES0_LANE0_USB 0x1
+/* J721S2 */
+
+#define J721S2_SERDES0_LANE0_EDP_LANE0 0x0
+#define J721S2_SERDES0_LANE0_PCIE1_LANE0 0x1
+#define J721S2_SERDES0_LANE0_IP3_UNUSED 0x2
+#define J721S2_SERDES0_LANE0_IP4_UNUSED 0x3
+
+#define J721S2_SERDES0_LANE1_EDP_LANE1 0x0
+#define J721S2_SERDES0_LANE1_PCIE1_LANE1 0x1
+#define J721S2_SERDES0_LANE1_USB 0x2
+#define J721S2_SERDES0_LANE1_IP4_UNUSED 0x3
+
+#define J721S2_SERDES0_LANE2_EDP_LANE2 0x0
+#define J721S2_SERDES0_LANE2_PCIE1_LANE2 0x1
+#define J721S2_SERDES0_LANE2_IP3_UNUSED 0x2
+#define J721S2_SERDES0_LANE2_IP4_UNUSED 0x3
+
+#define J721S2_SERDES0_LANE3_EDP_LANE3 0x0
+#define J721S2_SERDES0_LANE3_PCIE1_LANE3 0x1
+#define J721S2_SERDES0_LANE3_USB 0x2
+#define J721S2_SERDES0_LANE3_IP4_UNUSED 0x3
+
#endif /* _DT_BINDINGS_MUX_TI_SERDES */
--
2.20.1
From: Aswath Govindraju <[email protected]>
In some cases, it is required to provide the state to which the mux
controller has to be set to, from the consumer device tree node. Document
the property mux-states that can be used for adding this support.
Signed-off-by: Aswath Govindraju <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Peter Rosin <[email protected]>
---
.../devicetree/bindings/mux/gpio-mux.yaml | 11 ++++++--
.../devicetree/bindings/mux/mux-consumer.yaml | 21 +++++++++++++++
.../bindings/mux/mux-controller.yaml | 26 ++++++++++++++++++-
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.yaml b/Documentation/devicetree/bindings/mux/gpio-mux.yaml
index 0a7c8d64981a..ee4de9fbaf4d 100644
--- a/Documentation/devicetree/bindings/mux/gpio-mux.yaml
+++ b/Documentation/devicetree/bindings/mux/gpio-mux.yaml
@@ -26,7 +26,10 @@ properties:
List of gpios used to control the multiplexer, least significant bit first.
'#mux-control-cells':
- const: 0
+ enum: [ 0, 1 ]
+
+ '#mux-state-cells':
+ enum: [ 1, 2 ]
idle-state:
default: -1
@@ -34,7 +37,11 @@ properties:
required:
- compatible
- mux-gpios
- - "#mux-control-cells"
+anyOf:
+ - required:
+ - "#mux-control-cells"
+ - required:
+ - "#mux-state-cells"
additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mux/mux-consumer.yaml b/Documentation/devicetree/bindings/mux/mux-consumer.yaml
index 7af93298ab5c..d3d854967359 100644
--- a/Documentation/devicetree/bindings/mux/mux-consumer.yaml
+++ b/Documentation/devicetree/bindings/mux/mux-consumer.yaml
@@ -25,6 +25,17 @@ description: |
strings to label each of the mux controllers listed in the "mux-controls"
property.
+ If it is required to provide the state that the mux controller needs to
+ be set to, the property "mux-states" must be used. An optional property
+ "mux-state-names" can be used to provide a list of strings, to label
+ each of the multiplixer states listed in the "mux-states" property.
+
+ Properties "mux-controls" and "mux-states" can be used depending on how
+ the consumers want to control the mux controller. If the consumer needs
+ needs to set multiple states in a mux controller, then property
+ "mux-controls" can be used. If the consumer needs to set the mux
+ controller to a given state then property "mux-states" can be used.
+
mux-ctrl-specifier typically encodes the chip-relative mux controller number.
If the mux controller chip only provides a single mux controller, the
mux-ctrl-specifier can typically be left out.
@@ -35,12 +46,22 @@ properties:
mux-controls:
$ref: /schemas/types.yaml#/definitions/phandle-array
+ mux-states:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+
mux-control-names:
description:
Devices that use more than a single mux controller can use the
"mux-control-names" property to map the name of the requested mux
controller to an index into the list given by the "mux-controls" property.
+ mux-state-names:
+ description:
+ Devices that use more than a single multiplexer state can use the
+ "mux-state-names" property to map the name of the requested mux
+ controller to an index into the list given by the "mux-states"
+ property.
+
additionalProperties: true
...
diff --git a/Documentation/devicetree/bindings/mux/mux-controller.yaml b/Documentation/devicetree/bindings/mux/mux-controller.yaml
index 736a84c3b6a5..c855fbad3884 100644
--- a/Documentation/devicetree/bindings/mux/mux-controller.yaml
+++ b/Documentation/devicetree/bindings/mux/mux-controller.yaml
@@ -25,7 +25,9 @@ description: |
--------------------
Mux controller nodes must specify the number of cells used for the
- specifier using the '#mux-control-cells' property.
+ specifier using the '#mux-control-cells' or '#mux-state-cells' property.
+ The value of '#mux-state-cells' will always be one greater than the value
+ of '#mux-control-cells'.
Optionally, mux controller nodes can also specify the state the mux should
have when it is idle. The idle-state property is used for this. If the
@@ -67,6 +69,8 @@ select:
pattern: '^mux-controller'
- required:
- '#mux-control-cells'
+ - required:
+ - '#mux-state-cells'
properties:
$nodename:
@@ -75,6 +79,9 @@ properties:
'#mux-control-cells':
enum: [ 0, 1 ]
+ '#mux-state-cells':
+ enum: [ 1, 2 ]
+
idle-state:
$ref: /schemas/types.yaml#/definitions/int32
minimum: -2
@@ -179,4 +186,21 @@ examples:
};
};
};
+
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ mux1: mux-controller {
+ compatible = "gpio-mux";
+ #mux-state-cells = <1>;
+ mux-gpios = <&exp_som 2 GPIO_ACTIVE_HIGH>;
+ };
+
+ transceiver4: can-phy4 {
+ compatible = "ti,tcan1042";
+ #phy-cells = <0>;
+ max-bitrate = <5000000>;
+ standby-gpios = <&exp_som 7 GPIO_ACTIVE_HIGH>;
+ mux-states = <&mux1 1>;
+ };
...
--
2.20.1
From: Aswath Govindraju <[email protected]>
In some cases, we might need to provide the state of the mux to be set for
the operation of a given peripheral. Therefore, pass this information using
mux-states property.
Signed-off-by: Aswath Govindraju <[email protected]>
Signed-off-by: Peter Rosin <[email protected]> (minor edits)
---
.../driver-api/driver-model/devres.rst | 1 +
drivers/mux/core.c | 220 ++++++++++++++++--
include/linux/mux/consumer.h | 19 ++
3 files changed, 219 insertions(+), 21 deletions(-)
diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index 148e19381b79..5018403fe82f 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -368,6 +368,7 @@ MUX
devm_mux_chip_alloc()
devm_mux_chip_register()
devm_mux_control_get()
+ devm_mux_state_get()
NET
devm_alloc_etherdev()
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 22f4709768d1..7d38e7c0c02e 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -29,6 +29,20 @@
*/
#define MUX_CACHE_UNKNOWN MUX_IDLE_AS_IS
+/**
+ * struct mux_state - Represents a mux controller state specific to a given
+ * consumer.
+ * @mux: Pointer to a mux controller.
+ * @state State of the mux to be selected.
+ *
+ * This structure is specific to the consumer that acquires it and has
+ * information specific to that consumer.
+ */
+struct mux_state {
+ struct mux_control *mux;
+ unsigned int state;
+};
+
static struct class mux_class = {
.name = "mux",
.owner = THIS_MODULE,
@@ -341,7 +355,8 @@ static void mux_control_delay(struct mux_control *mux, unsigned int delay_us)
* On successfully selecting the mux-control state, it will be locked until
* there is a call to mux_control_deselect(). If the mux-control is already
* selected when mux_control_select() is called, the caller will be blocked
- * until mux_control_deselect() is called (by someone else).
+ * until mux_control_deselect() or mux_state_deselect() is called (by someone
+ * else).
*
* Therefore, make sure to call mux_control_deselect() when the operation is
* complete and the mux-control is free for others to use, but do not call
@@ -370,6 +385,30 @@ int mux_control_select_delay(struct mux_control *mux, unsigned int state,
}
EXPORT_SYMBOL_GPL(mux_control_select_delay);
+/**
+ * mux_state_select_delay() - Select the given multiplexer state.
+ * @mstate: The mux-state to select.
+ * @delay_us: The time to delay (in microseconds) if the mux state is changed.
+ *
+ * On successfully selecting the mux-state, its mux-control will be locked
+ * until there is a call to mux_state_deselect(). If the mux-control is already
+ * selected when mux_state_select() is called, the caller will be blocked
+ * until mux_state_deselect() or mux_control_deselect() is called (by someone
+ * else).
+ *
+ * Therefore, make sure to call mux_state_deselect() when the operation is
+ * complete and the mux-control is free for others to use, but do not call
+ * mux_state_deselect() if mux_state_select() fails.
+ *
+ * Return: 0 when the mux-state has been selected or a negative
+ * errno on error.
+ */
+int mux_state_select_delay(struct mux_state *mstate, unsigned int delay_us)
+{
+ return mux_control_select_delay(mstate->mux, mstate->state, delay_us);
+}
+EXPORT_SYMBOL_GPL(mux_state_select_delay);
+
/**
* mux_control_try_select_delay() - Try to select the given multiplexer state.
* @mux: The mux-control to request a change of state from.
@@ -405,6 +444,27 @@ int mux_control_try_select_delay(struct mux_control *mux, unsigned int state,
}
EXPORT_SYMBOL_GPL(mux_control_try_select_delay);
+/**
+ * mux_state_try_select_delay() - Try to select the given multiplexer state.
+ * @mstate: The mux-state to select.
+ * @delay_us: The time to delay (in microseconds) if the mux state is changed.
+ *
+ * On successfully selecting the mux-state, its mux-control will be locked
+ * until mux_state_deselect() is called.
+ *
+ * Therefore, make sure to call mux_state_deselect() when the operation is
+ * complete and the mux-control is free for others to use, but do not call
+ * mux_state_deselect() if mux_state_try_select() fails.
+ *
+ * Return: 0 when the mux-state has been selected or a negative errno on
+ * error. Specifically -EBUSY if the mux-control is contended.
+ */
+int mux_state_try_select_delay(struct mux_state *mstate, unsigned int delay_us)
+{
+ return mux_control_try_select_delay(mstate->mux, mstate->state, delay_us);
+}
+EXPORT_SYMBOL_GPL(mux_state_try_select_delay);
+
/**
* mux_control_deselect() - Deselect the previously selected multiplexer state.
* @mux: The mux-control to deselect.
@@ -431,6 +491,24 @@ int mux_control_deselect(struct mux_control *mux)
}
EXPORT_SYMBOL_GPL(mux_control_deselect);
+/**
+ * mux_state_deselect() - Deselect the previously selected multiplexer state.
+ * @mstate: The mux-state to deselect.
+ *
+ * It is required that a single call is made to mux_state_deselect() for
+ * each and every successful call made to either of mux_state_select() or
+ * mux_state_try_select().
+ *
+ * Return: 0 on success and a negative errno on error. An error can only
+ * occur if the mux has an idle state. Note that even if an error occurs, the
+ * mux-control is unlocked and is thus free for the next access.
+ */
+int mux_state_deselect(struct mux_state *mstate)
+{
+ return mux_control_deselect(mstate->mux);
+}
+EXPORT_SYMBOL_GPL(mux_state_deselect);
+
/* Note this function returns a reference to the mux_chip dev. */
static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
{
@@ -441,14 +519,17 @@ static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
return dev ? to_mux_chip(dev) : NULL;
}
-/**
- * mux_control_get() - Get the mux-control for a device.
+/*
+ * mux_get() - Get the mux-control for a device.
* @dev: The device that needs a mux-control.
* @mux_name: The name identifying the mux-control.
+ * @state: Pointer to where the requested state is returned, or NULL when
+ * the required multiplexer states are handled by other means.
*
* Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
*/
-struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+static struct mux_control *mux_get(struct device *dev, const char *mux_name,
+ unsigned int *state)
{
struct device_node *np = dev->of_node;
struct of_phandle_args args;
@@ -458,8 +539,12 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
int ret;
if (mux_name) {
- index = of_property_match_string(np, "mux-control-names",
- mux_name);
+ if (state)
+ index = of_property_match_string(np, "mux-state-names",
+ mux_name);
+ else
+ index = of_property_match_string(np, "mux-control-names",
+ mux_name);
if (index < 0) {
dev_err(dev, "mux controller '%s' not found\n",
mux_name);
@@ -467,12 +552,17 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
}
}
- ret = of_parse_phandle_with_args(np,
- "mux-controls", "#mux-control-cells",
- index, &args);
+ if (state)
+ ret = of_parse_phandle_with_args(np,
+ "mux-states", "#mux-state-cells",
+ index, &args);
+ else
+ ret = of_parse_phandle_with_args(np,
+ "mux-controls", "#mux-control-cells",
+ index, &args);
if (ret) {
- dev_err(dev, "%pOF: failed to get mux-control %s(%i)\n",
- np, mux_name ?: "", index);
+ dev_err(dev, "%pOF: failed to get mux-%s %s(%i)\n",
+ np, state ? "state" : "control", mux_name ?: "", index);
return ERR_PTR(ret);
}
@@ -481,17 +571,35 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
if (!mux_chip)
return ERR_PTR(-EPROBE_DEFER);
- if (args.args_count > 1 ||
- (!args.args_count && (mux_chip->controllers > 1))) {
- dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
- np, args.np);
- put_device(&mux_chip->dev);
- return ERR_PTR(-EINVAL);
- }
-
controller = 0;
- if (args.args_count)
- controller = args.args[0];
+ if (state) {
+ if (args.args_count > 2 || args.args_count == 0 ||
+ (args.args_count < 2 && mux_chip->controllers > 1)) {
+ dev_err(dev, "%pOF: wrong #mux-state-cells for %pOF\n",
+ np, args.np);
+ put_device(&mux_chip->dev);
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (args.args_count == 2) {
+ controller = args.args[0];
+ *state = args.args[1];
+ } else {
+ *state = args.args[0];
+ }
+
+ } else {
+ if (args.args_count > 1 ||
+ (!args.args_count && mux_chip->controllers > 1)) {
+ dev_err(dev, "%pOF: wrong #mux-control-cells for %pOF\n",
+ np, args.np);
+ put_device(&mux_chip->dev);
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (args.args_count)
+ controller = args.args[0];
+ }
if (controller >= mux_chip->controllers) {
dev_err(dev, "%pOF: bad mux controller %u specified in %pOF\n",
@@ -502,6 +610,18 @@ struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
return &mux_chip->mux[controller];
}
+
+/**
+ * mux_control_get() - Get the mux-control for a device.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
+ */
+struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
+{
+ return mux_get(dev, mux_name, NULL);
+}
EXPORT_SYMBOL_GPL(mux_control_get);
/**
@@ -553,6 +673,64 @@ struct mux_control *devm_mux_control_get(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_mux_control_get);
+/**
+ * mux_state_put() - Put away the mux-state for good.
+ * @mstate: The mux-state to put away.
+ *
+ * mux_state_put() reverses the effects of mux_state_get().
+ */
+void mux_state_put(struct mux_state *mstate)
+{
+ mux_control_put(mstate->mux);
+ kfree(mstate);
+}
+EXPORT_SYMBOL_GPL(mux_state_put);
+
+static void devm_mux_state_release(struct device *dev, void *res)
+{
+ struct mux_state *mstate = *(struct mux_state **)res;
+
+ mux_state_put(mstate);
+}
+
+/**
+ * devm_mux_state_get() - Get the mux-state for a device, with resource
+ * management.
+ * @dev: The device that needs a mux-control.
+ * @mux_name: The name identifying the mux-control.
+ *
+ * Return: Pointer to the mux-state, or an ERR_PTR with a negative errno.
+ */
+struct mux_state *devm_mux_state_get(struct device *dev,
+ const char *mux_name)
+{
+ struct mux_state **ptr, *mstate;
+ struct mux_control *mux_ctrl;
+ int state;
+
+ mstate = devm_kzalloc(dev, sizeof(struct mux_state), GFP_KERNEL);
+ if (!mstate)
+ return ERR_PTR(-ENOMEM);
+
+ ptr = devres_alloc(devm_mux_state_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ mux_ctrl = mux_get(dev, mux_name, &state);
+ if (IS_ERR(mux_ctrl)) {
+ devres_free(ptr);
+ return (struct mux_state *)mux_ctrl;
+ }
+
+ mstate->mux = mux_ctrl;
+ mstate->state = state;
+ *ptr = mstate;
+ devres_add(dev, ptr);
+
+ return mstate;
+}
+EXPORT_SYMBOL_GPL(devm_mux_state_get);
+
/*
* Using subsys_initcall instead of module_init here to try to ensure - for
* the non-modular case - that the subsystem is initialized when mux consumers
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index 7a09b040ac39..babf2a744056 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -14,14 +14,19 @@
struct device;
struct mux_control;
+struct mux_state;
unsigned int mux_control_states(struct mux_control *mux);
int __must_check mux_control_select_delay(struct mux_control *mux,
unsigned int state,
unsigned int delay_us);
+int __must_check mux_state_select_delay(struct mux_state *mstate,
+ unsigned int delay_us);
int __must_check mux_control_try_select_delay(struct mux_control *mux,
unsigned int state,
unsigned int delay_us);
+int __must_check mux_state_try_select_delay(struct mux_state *mstate,
+ unsigned int delay_us);
static inline int __must_check mux_control_select(struct mux_control *mux,
unsigned int state)
@@ -29,18 +34,32 @@ static inline int __must_check mux_control_select(struct mux_control *mux,
return mux_control_select_delay(mux, state, 0);
}
+static inline int __must_check mux_state_select(struct mux_state *mstate)
+{
+ return mux_state_select_delay(mstate, 0);
+}
+
static inline int __must_check mux_control_try_select(struct mux_control *mux,
unsigned int state)
{
return mux_control_try_select_delay(mux, state, 0);
}
+static inline int __must_check mux_state_try_select(struct mux_state *mstate)
+{
+ return mux_state_try_select_delay(mstate, 0);
+}
+
int mux_control_deselect(struct mux_control *mux);
+int mux_state_deselect(struct mux_state *mstate);
struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
void mux_control_put(struct mux_control *mux);
+void mux_state_put(struct mux_state *mstate);
struct mux_control *devm_mux_control_get(struct device *dev,
const char *mux_name);
+struct mux_state *devm_mux_state_get(struct device *dev,
+ const char *mux_name);
#endif /* _LINUX_MUX_CONSUMER_H */
--
2.20.1
From: Peter Rosin <[email protected]>
And implement devm_mux_state_get in terms of the new function.
Tested-by: Aswath Govindraju <[email protected]>
Signed-off-by: Peter Rosin <[email protected]>
---
drivers/mux/core.c | 41 ++++++++++++++++++++++++++----------
include/linux/mux/consumer.h | 1 +
2 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 7d38e7c0c02e..90073ce01539 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -673,6 +673,33 @@ struct mux_control *devm_mux_control_get(struct device *dev,
}
EXPORT_SYMBOL_GPL(devm_mux_control_get);
+/**
+ * mux_state_get() - Get the mux-state for a device.
+ * @dev: The device that needs a mux-state.
+ * @mux_name: The name identifying the mux-state.
+ *
+ * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno.
+ */
+struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
+{
+ struct mux_state *mstate;
+
+ mstate = kzalloc(sizeof(*mstate), GFP_KERNEL);
+ if (!mstate)
+ return ERR_PTR(-ENOMEM);
+
+ mstate->mux = mux_get(dev, mux_name, &mstate->state);
+ if (IS_ERR(mstate->mux)) {
+ int err = PTR_ERR(mstate->mux);
+
+ kfree(mstate);
+ return ERR_PTR(err);
+ }
+
+ return mstate;
+}
+EXPORT_SYMBOL_GPL(mux_state_get);
+
/**
* mux_state_put() - Put away the mux-state for good.
* @mstate: The mux-state to put away.
@@ -705,25 +732,17 @@ struct mux_state *devm_mux_state_get(struct device *dev,
const char *mux_name)
{
struct mux_state **ptr, *mstate;
- struct mux_control *mux_ctrl;
- int state;
-
- mstate = devm_kzalloc(dev, sizeof(struct mux_state), GFP_KERNEL);
- if (!mstate)
- return ERR_PTR(-ENOMEM);
ptr = devres_alloc(devm_mux_state_release, sizeof(*ptr), GFP_KERNEL);
if (!ptr)
return ERR_PTR(-ENOMEM);
- mux_ctrl = mux_get(dev, mux_name, &state);
- if (IS_ERR(mux_ctrl)) {
+ mstate = mux_state_get(dev, mux_name);
+ if (IS_ERR(mstate)) {
devres_free(ptr);
- return (struct mux_state *)mux_ctrl;
+ return mstate;
}
- mstate->mux = mux_ctrl;
- mstate->state = state;
*ptr = mstate;
devres_add(dev, ptr);
diff --git a/include/linux/mux/consumer.h b/include/linux/mux/consumer.h
index babf2a744056..944678604549 100644
--- a/include/linux/mux/consumer.h
+++ b/include/linux/mux/consumer.h
@@ -54,6 +54,7 @@ int mux_control_deselect(struct mux_control *mux);
int mux_state_deselect(struct mux_state *mstate);
struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
+struct mux_state *mux_state_get(struct device *dev, const char *mux_name);
void mux_control_put(struct mux_control *mux);
void mux_state_put(struct mux_state *mstate);
--
2.20.1
Signed-off-by: Peter Rosin <[email protected]>
---
drivers/mux/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index 90073ce01539..fc339b0064e9 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -416,7 +416,7 @@ EXPORT_SYMBOL_GPL(mux_state_select_delay);
* @delay_us: The time to delay (in microseconds) if the mux state is changed.
*
* On successfully selecting the mux-control state, it will be locked until
- * mux_control_deselect() called.
+ * mux_control_deselect() is called.
*
* Therefore, make sure to call mux_control_deselect() when the operation is
* complete and the mux-control is free for others to use, but do not call
--
2.20.1
From: Yang Li <[email protected]>
A warning is reported because a colon was dropped, it is found by running
scripts/kernel-doc, which is caused by using 'make W=1'.
drivers/mux/core.c:44: warning: Function parameter or member 'state' not
described in 'mux_state'
Reported-by: Abaci Robot <[email protected]>
Signed-off-by: Yang Li <[email protected]>
Signed-off-by: Peter Rosin <[email protected]>
---
drivers/mux/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mux/core.c b/drivers/mux/core.c
index fc339b0064e9..11a5c488c812 100644
--- a/drivers/mux/core.c
+++ b/drivers/mux/core.c
@@ -33,7 +33,7 @@
* struct mux_state - Represents a mux controller state specific to a given
* consumer.
* @mux: Pointer to a mux controller.
- * @state State of the mux to be selected.
+ * @state: State of the mux to be selected.
*
* This structure is specific to the consumer that acquires it and has
* information specific to that consumer.
--
2.20.1
On Sun, Jan 02, 2022 at 11:38:45PM +0100, Peter Rosin wrote:
>
> Signed-off-by: Peter Rosin <[email protected]>
> ---
> drivers/mux/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I really do not like to take changes with no changelog text at all :(
thanks,
greg k-h
On Sun, Jan 02, 2022 at 11:38:36PM +0100, Peter Rosin wrote:
> From: Peter Rosin <[email protected]>
>
> And implement devm_mux_state_get in terms of the new function.
>
> Tested-by: Aswath Govindraju <[email protected]>
> Signed-off-by: Peter Rosin <[email protected]>
> ---
> drivers/mux/core.c | 41 ++++++++++++++++++++++++++----------
> include/linux/mux/consumer.h | 1 +
> 2 files changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> index 7d38e7c0c02e..90073ce01539 100644
> --- a/drivers/mux/core.c
> +++ b/drivers/mux/core.c
> @@ -673,6 +673,33 @@ struct mux_control *devm_mux_control_get(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(devm_mux_control_get);
>
> +/**
> + * mux_state_get() - Get the mux-state for a device.
> + * @dev: The device that needs a mux-state.
> + * @mux_name: The name identifying the mux-state.
> + *
> + * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno.
> + */
> +struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
> +{
> + struct mux_state *mstate;
> +
> + mstate = kzalloc(sizeof(*mstate), GFP_KERNEL);
> + if (!mstate)
> + return ERR_PTR(-ENOMEM);
> +
> + mstate->mux = mux_get(dev, mux_name, &mstate->state);
will this build? I haven't applied it but mux_get() in my tree right
now is defined as:
static inline void mux_get(struct gsm_mux *gsm)
> + if (IS_ERR(mstate->mux)) {
> + int err = PTR_ERR(mstate->mux);
> +
> + kfree(mstate);
> + return ERR_PTR(err);
> + }
> +
> + return mstate;
> +}
> +EXPORT_SYMBOL_GPL(mux_state_get);
No need to export it or make it global if no one is using it, right?
Also, who frees this new memory you just allocated?
> +
> /**
> * mux_state_put() - Put away the mux-state for good.
> * @mstate: The mux-state to put away.
> @@ -705,25 +732,17 @@ struct mux_state *devm_mux_state_get(struct device *dev,
> const char *mux_name)
> {
> struct mux_state **ptr, *mstate;
> - struct mux_control *mux_ctrl;
> - int state;
> -
> - mstate = devm_kzalloc(dev, sizeof(struct mux_state), GFP_KERNEL);
> - if (!mstate)
> - return ERR_PTR(-ENOMEM);
Before this state memory was here, assigned to the device, so it was
freed when the device was unbound. I'm missing where this now
happens...
thanks,
greg k-h
On Sun, Jan 02, 2022 at 11:38:29PM +0100, Peter Rosin wrote:
> From: Aswath Govindraju <[email protected]>
>
> In some cases, we might need to provide the state of the mux to be set for
> the operation of a given peripheral. Therefore, pass this information using
> mux-states property.
Where is the user of this new function?
thanks,
greg k-h
On Sun, Jan 02, 2022 at 11:34:12PM +0100, Peter Rosin wrote:
> Hi Greg,
>
> These have been in -next since around x-mas, and hopefully it's not too
> late to push them for the upcoming window.
>
> Patch 5/6 fails check-patch due to lacking a message body, because I
> forgot to actually check that and now I can't find it in me to re-spin
> for that seemingly insignificant issue. If it really is a no-no, then
> please just drop that patch.
It's a no-go, please take a break and find it in you for the next time
you resend this series :)
I'll take patches 1 and 2, the rest need work.
thanks,
greg k-h
Hi Greg,
On 03/01/22 6:13 pm, Greg Kroah-Hartman wrote:
> On Sun, Jan 02, 2022 at 11:38:29PM +0100, Peter Rosin wrote:
>> From: Aswath Govindraju <[email protected]>
>>
>> In some cases, we might need to provide the state of the mux to be set for
>> the operation of a given peripheral. Therefore, pass this information using
>> mux-states property.
>
> Where is the user of this new function?
>
This function will be used by the following patch series,
https://lore.kernel.org/lkml/[email protected]/t/
Since the above has a dependency on this patch series, it is planned to
be merged after this series.
Thanks,
Aswath
> thanks,
>
> greg k-h
>
On Mon, Jan 03, 2022 at 06:35:52PM +0530, Aswath Govindraju wrote:
> Hi Greg,
>
> On 03/01/22 6:13 pm, Greg Kroah-Hartman wrote:
> > On Sun, Jan 02, 2022 at 11:38:29PM +0100, Peter Rosin wrote:
> >> From: Aswath Govindraju <[email protected]>
> >>
> >> In some cases, we might need to provide the state of the mux to be set for
> >> the operation of a given peripheral. Therefore, pass this information using
> >> mux-states property.
> >
> > Where is the user of this new function?
> >
>
> This function will be used by the following patch series,
>
> https://lore.kernel.org/lkml/[email protected]/t/
>
> Since the above has a dependency on this patch series, it is planned to
> be merged after this series.
Please provide the users of new functions as part of the same series,
otherwise it is impossible to determine how the functions are used, or
if they are even used at all.
We try to not add functions with no in-tree users.
thanks,
greg k-h
Hi!
On 2022-01-03 13:42, Greg Kroah-Hartman wrote:
> On Sun, Jan 02, 2022 at 11:38:36PM +0100, Peter Rosin wrote:
>> From: Peter Rosin <[email protected]>
>>
>> And implement devm_mux_state_get in terms of the new function.
>>
>> Tested-by: Aswath Govindraju <[email protected]>
>> Signed-off-by: Peter Rosin <[email protected]>
>> ---
>> drivers/mux/core.c | 41 ++++++++++++++++++++++++++----------
>> include/linux/mux/consumer.h | 1 +
>> 2 files changed, 31 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
>> index 7d38e7c0c02e..90073ce01539 100644
>> --- a/drivers/mux/core.c
>> +++ b/drivers/mux/core.c
>> @@ -673,6 +673,33 @@ struct mux_control *devm_mux_control_get(struct device *dev,
>> }
>> EXPORT_SYMBOL_GPL(devm_mux_control_get);
>>
>> +/**
>> + * mux_state_get() - Get the mux-state for a device.
>> + * @dev: The device that needs a mux-state.
>> + * @mux_name: The name identifying the mux-state.
>> + *
>> + * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno.
>> + */
>> +struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
>> +{
>> + struct mux_state *mstate;
>> +
>> + mstate = kzalloc(sizeof(*mstate), GFP_KERNEL);
>> + if (!mstate)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + mstate->mux = mux_get(dev, mux_name, &mstate->state);
>
> will this build? I haven't applied it but mux_get() in my tree right
> now is defined as:
> static inline void mux_get(struct gsm_mux *gsm)
Yes it builds. As mentioned in the cover letter, the patches have been
in -next for a couple of weeks. The static definition you are pointing
at is from n_gsm.c (which does not seem to be #included by any other
file). This definition of mux_get is again static and in a .c file
(which is not #included by anything). Surely not a conflict?
>
>> + if (IS_ERR(mstate->mux)) {
>> + int err = PTR_ERR(mstate->mux);
>> +
>> + kfree(mstate);
>> + return ERR_PTR(err);
>> + }
>> +
>> + return mstate;
>> +}
>> +EXPORT_SYMBOL_GPL(mux_state_get);
>
> No need to export it or make it global if no one is using it, right?
This is support code, and I think it should be up to the consumers
to decide if managed interfaced should be used or not, and would
rather not push anyone to managed interfaces if that does not work
or does not fit for some reason.
> Also, who frees this new memory you just allocated?
mux_state_put
>
>> +
>> /**
>> * mux_state_put() - Put away the mux-state for good.
>> * @mstate: The mux-state to put away.
>> @@ -705,25 +732,17 @@ struct mux_state *devm_mux_state_get(struct device *dev,
>> const char *mux_name)
>> {
>> struct mux_state **ptr, *mstate;
>> - struct mux_control *mux_ctrl;
>> - int state;
>> -
>> - mstate = devm_kzalloc(dev, sizeof(struct mux_state), GFP_KERNEL);
>> - if (!mstate)
>> - return ERR_PTR(-ENOMEM);
>
> Before this state memory was here, assigned to the device, so it was
> freed when the device was unbound. I'm missing where this now
> happens...
Again, mux_state_put (via devm_mux_state_release).
Cheers,
Peter
On Mon, Jan 03, 2022 at 06:26:21PM +0100, Peter Rosin wrote:
> Hi!
>
> On 2022-01-03 13:42, Greg Kroah-Hartman wrote:
> > On Sun, Jan 02, 2022 at 11:38:36PM +0100, Peter Rosin wrote:
> >> From: Peter Rosin <[email protected]>
> >>
> >> And implement devm_mux_state_get in terms of the new function.
> >>
> >> Tested-by: Aswath Govindraju <[email protected]>
> >> Signed-off-by: Peter Rosin <[email protected]>
> >> ---
> >> drivers/mux/core.c | 41 ++++++++++++++++++++++++++----------
> >> include/linux/mux/consumer.h | 1 +
> >> 2 files changed, 31 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
> >> index 7d38e7c0c02e..90073ce01539 100644
> >> --- a/drivers/mux/core.c
> >> +++ b/drivers/mux/core.c
> >> @@ -673,6 +673,33 @@ struct mux_control *devm_mux_control_get(struct device *dev,
> >> }
> >> EXPORT_SYMBOL_GPL(devm_mux_control_get);
> >>
> >> +/**
> >> + * mux_state_get() - Get the mux-state for a device.
> >> + * @dev: The device that needs a mux-state.
> >> + * @mux_name: The name identifying the mux-state.
> >> + *
> >> + * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno.
> >> + */
> >> +struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
> >> +{
> >> + struct mux_state *mstate;
> >> +
> >> + mstate = kzalloc(sizeof(*mstate), GFP_KERNEL);
> >> + if (!mstate)
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + mstate->mux = mux_get(dev, mux_name, &mstate->state);
> >
> > will this build? I haven't applied it but mux_get() in my tree right
> > now is defined as:
> > static inline void mux_get(struct gsm_mux *gsm)
>
> Yes it builds. As mentioned in the cover letter, the patches have been
> in -next for a couple of weeks. The static definition you are pointing
> at is from n_gsm.c (which does not seem to be #included by any other
> file). This definition of mux_get is again static and in a .c file
> (which is not #included by anything). Surely not a conflict?
If it's static, no, it's fine, but I don't see it in this commit either?
I'm confused now, can you resend the remaining changes and I will review
them again?
thanks,
greg k-h
Hi!
On 2022-01-06 15:41, Greg Kroah-Hartman wrote:
> On Mon, Jan 03, 2022 at 06:26:21PM +0100, Peter Rosin wrote:
>> Hi!
>>
>> On 2022-01-03 13:42, Greg Kroah-Hartman wrote:
>>> On Sun, Jan 02, 2022 at 11:38:36PM +0100, Peter Rosin wrote:
>>>> From: Peter Rosin <[email protected]>
>>>>
>>>> And implement devm_mux_state_get in terms of the new function.
>>>>
>>>> Tested-by: Aswath Govindraju <[email protected]>
>>>> Signed-off-by: Peter Rosin <[email protected]>
>>>> ---
>>>> drivers/mux/core.c | 41 ++++++++++++++++++++++++++----------
>>>> include/linux/mux/consumer.h | 1 +
>>>> 2 files changed, 31 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/mux/core.c b/drivers/mux/core.c
>>>> index 7d38e7c0c02e..90073ce01539 100644
>>>> --- a/drivers/mux/core.c
>>>> +++ b/drivers/mux/core.c
>>>> @@ -673,6 +673,33 @@ struct mux_control *devm_mux_control_get(struct device *dev,
>>>> }
>>>> EXPORT_SYMBOL_GPL(devm_mux_control_get);
>>>>
>>>> +/**
>>>> + * mux_state_get() - Get the mux-state for a device.
>>>> + * @dev: The device that needs a mux-state.
>>>> + * @mux_name: The name identifying the mux-state.
>>>> + *
>>>> + * Return: A pointer to the mux-state, or an ERR_PTR with a negative errno.
>>>> + */
>>>> +struct mux_state *mux_state_get(struct device *dev, const char *mux_name)
>>>> +{
>>>> + struct mux_state *mstate;
>>>> +
>>>> + mstate = kzalloc(sizeof(*mstate), GFP_KERNEL);
>>>> + if (!mstate)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + mstate->mux = mux_get(dev, mux_name, &mstate->state);
>>>
>>> will this build? I haven't applied it but mux_get() in my tree right
>>> now is defined as:
>>> static inline void mux_get(struct gsm_mux *gsm)
>>
>> Yes it builds. As mentioned in the cover letter, the patches have been
>> in -next for a couple of weeks. The static definition you are pointing
>> at is from n_gsm.c (which does not seem to be #included by any other
>> file). This definition of mux_get is again static and in a .c file
>> (which is not #included by anything). Surely not a conflict?
>
> If it's static, no, it's fine, but I don't see it in this commit either?
>
> I'm confused now,
Apparently :-)
The static drivers/mux/core.c:mux_get() is not in your tree because it was
introduced in patch 3/6. That patch refactored the existing mux_control_get()
into a new static helper function mux_get() with two wrappers -- the old
mux_control_get() that preserves the preexisting interface and the new
devm_mux_state_get(). mux_control_get() was always in turn wrapped by
devm_mux_control_get(), while patch 3/6 failed to add a similar double
wrapping with an intermediate mux_state_get(). Instead it wrapped mux_get()
directly.
I didn't notice that mux_state_get() was missing until after a couple of
rounds of review with Aswath, and didn't want go for another round when it
was me who had made a mistake, and instead just fixed it with a commit of
my own.
Maybe you thought "the new function" that this commit message speaks about
was mux_get() (which was new in 3/6, but no longer "new" here in 4/6), when
in fact it refers to mux_state_get()?
> can you resend the remaining changes and I will review
> them again?
On it.
Cheers,
Peter