Re: [PATCH v3 1/3] mfd: add support for Cypress CYUSBS234 USB Serial Bridge controller

On Thu, Oct 09, 2014 at 11:59:50AM +0100, Lee Jones wrote:

On Thu, 09 Oct 2014, Johan Hovold wrote:

quoted

On Thu, Oct 09, 2014 at 08:40:29AM +0100, Lee Jones wrote:

quoted

On Mon, 06 Oct 2014, Muthu Mani wrote:

quoted

quoted

quoted

diff --git a/include/linux/mfd/cyusbs23x.h b/include/linux/mfd/cyusbs23x.h

quoted

quoted

+/* Serial interfaces (I2C, SPI, UART) differ in interface subclass */
+enum cy_scb_modes {
+	CY_USBS_SCB_DISABLED = 0,
+	CY_USBS_SCB_UART = 1,
+	CY_USBS_SCB_SPI = 2,
+	CY_USBS_SCB_I2C = 3

No need to number these.

As it's not an arbitrary enumeration, I think they should be initialised
explicitly.

No need.  You are protected by the C Standard:

6.7.2.2 Enumeration specifiers

"If the first enumerator has no =, the value of its enumeration
constant is 0. Each subsequent enumerator with no = defines its
enumeration constant as the value of the constant expression obtained
by adding 1 to the value of the previous enumeration constant."

There's nothing arbitrary about that.

I obviously wasn't suggesting that the definition of an enum (and the
values of its constants) in c was arbitrary.

My point was that the values of the USB interface subclasses (defined
through the enum) are not arbitrary. In this case they just happen to be
zero-based and consecutive. You cannot reorder, or remove an unused
item, without breaking the driver. By initialising each constant
explicitly this would become apparent.

Using preprocessor defines could be an alternative if you really do not
like initialised enumeration constants.

quoted

They could be defined in the mfd driver though, as they only
appear to be needed during probe.

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html