Re: [PATCH v3 7/9] rcar-phy: add platform data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10-04-2013 13:00, Felipe Balbi wrote:

Currently the driver hard-codes USBPCTRL0 register to 0.  It is wrong since this
register contains board-specific USB ports configuration and so its value should
be somehow passed via the platform data.  Add <linux/usb/rcar-phy.h> file with
the USBPCTRL0 bit #define's and 'struct rcar_phy_platform_data' containing the
value to be set by the driver to that register.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>
Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
Acked-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>

---
Changes since version 2:
- added #include <linux/types.h>;
- added ACKs from Simon Horman and Kuninori Morimoto.

  include/linux/usb/rcar-phy.h |   40 ++++++++++++++++++++++++++++++++++++++++
  1 file changed, 40 insertions(+)

Index: renesas/include/linux/usb/rcar-phy.h
===================================================================
--- /dev/null
+++ renesas/include/linux/usb/rcar-phy.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2013 Renesas Solutions Corp.
+ * Copyright (C) 2013 Cogent Embedded, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __RCAR_PHY_H
+#define __RCAR_PHY_H
+
+#include <linux/bitops.h>
+#include <linux/types.h>
+
+/* USBPCTRL0 register bits */
+#define USBPCTRL0_OVC2	BIT(10)	/* Switches the OVC input pin for port 2: */
+				/* 1: USB_OVC2, 0: OVC2			*/
+#define USBPCTRL0_OVC1_VBUS1 BIT(9) /* Switches the OVC input pin for port 1: */
+				/* 1: USB_OVC1, 0: OVC1/VBUS1		*/
+#define USBPCTRL0_OVC0	BIT(8)	/* Switches the OVC input pin for port 0: */
+				/* 1: USB_OVC0 pin, 0: OVC0		*/
+#define USBPCTRL0_OVC2_ACT BIT(6) /* Host mode: OVC2 polarity:		*/
+				/* 1: active-high, 0: active-low	*/
+				/* Function mode: be sure to set to 1	*/
+#define USBPCTRL0_PENC	BIT(4)	/* Function mode: output level of PENC1 pin: */
+				/* 1: high, 0: low			*/
+#define USBPCTRL0_OVC0_ACT BIT(3) /* Host mode: OVC0 polarity:		*/
+				/* 1: active-high, 0: active-low	*/
+#define USBPCTRL0_OVC1_ACT BIT(1) /* Host mode: OVC1 polarity:		*/
+				/* 1: active-high, 0: active-low	*/
+				/* Function mode: be sure to set to 1	*/
+#define USBPCTRL0_PORT1	BIT(0)	/* Selects port 1 mode:			*/
+				/* 1: function, 0: host			*/
+
+struct rcar_phy_platform_data {
+	u32 usbpctrl0;		/* USBPCTRL0 register value */
+};

looks really wrong to me to pass register contents via pdata. You should
pass flags which would aid the driver into figuring out how to program
that register.

That was my first thought (and implementation) but this didn't look good either (and even worse with the device tree approach) as we couldn't come up with the clear names for the bitfields. Also, not all these bits are present in R8A7778 support for which I'm adding in the later patchset. Besides, IMO this little differs from having a flags field with the flags bits #define'd beforehand. Or did you mean that I should have surely used *bool* bitfields?

WBR, Sergei

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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux