On 19/12/24 15:57, Krzysztof Kozlowski wrote:
On 19/12/2024 14:36, Richard Fitzgerald wrote:
On 19/12/24 13:16, Krzysztof Kozlowski wrote:
On 19/12/2024 13:59, Charles Keepax wrote:
On Thu, Dec 19, 2024 at 12:39:38PM +0100, Krzysztof Kozlowski wrote:
On 19/12/2024 12:02, Charles Keepax wrote:
On Thu, Dec 19, 2024 at 09:51:00AM +0100, Krzysztof Kozlowski wrote:
On Wed, Dec 18, 2024 at 08:46:30PM -0600, Paul Handrigan wrote:
+/* CS2600 Auxiliary Output */
+#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0
+#define CS2600_AUX_OUTPUT_PHASE_UNLOCK 1
+#define CS2600_AUX_OUTPUT_NO_CLKIN 2
I still don't see why these three are supposed to be bindings. Drop
them.
In a binding one would presumably do:
cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>;
Apologies but I don't quite understand what you mean by the values
are not used in the binding? The driver reads the property and sets
There is no user of these defines, so not a binding.
the pin to have the appropriate function. Admittedly one could drop
It's not a proof that this is a binding.
the defines and then DTS would just have to do:
cirrus,aux-output-source = <0>;
But that feels a bit less helpful when reading the binding.
Binding and being helpful are two different things. This to be the
binding, it has to be used as a binding, so some translation layer
between driver and DTS. It must have an user in DTS. I keep repeating
this over and over...
Apologies, but I not sure I totally follow this, and apologies if
you have already explained this are there some docs I can look
at?
I think you are saying because these defines merely represent the
valid values for a device tree property and are not translated
into different values you can't put defines for them in the binding
header?
So this would not be allowed:
#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 0
cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>;
device_property_read_u32(dev, "cirrus,aux-output-source", &val);
regmap_write(regmap, CS2600_OUTPUT_CFG2, val);
But this would be fine:
#define CS2600_AUX_OUTPUT_FREQ_UNLOCK 1
cirrus,aux-output-source = <CS2600_AUX_OUTPUT_FREQ_UNLOCK>;
device_property_read_u32(dev, "cirrus,aux-output-source", &val);
switch (val) {
case CS2600_AUX_OUTPUT_FREQ_UNLOCK:
regmap_write(regmap, CS2600_OUTPUT_CFG2, 0);
}
And this would also be fine?
cirrus,aux-output-source = <0>;
device_property_read_u32(dev, "cirrus,aux-output-source", &val);
regmap_write(regmap, CS2600_OUTPUT_CFG2, val);
Yes. If you want to use in DTS user-readable values, then use string.
I don't understand this. Why should we have to use a string value for
something that only needs a simple integer value? Why can't we define
constants with meaningful names?
You can and you will find plenty examples of this, but as I explained
earlier - this is not a binding. We avoid defining as a binding
something which is not a binding.
What does that mean?
Perhaps if you clearly explained what the problem is and what you want
us to change instead of making cryptic statements like "this is not a
binding" we wouldn't have to waste all this time exchanging emails that
aren't getting anywhere.
You didn't explain earlier. You typed some words earlier, but they
failed to explain, and you are continuing to fail to explain.
Best regards,
Krzysztof
