Re: [PATCH v2 3/7] usb: chipidea: Support generic usb extcon

2018-08-29 Thread Loic Poulain
Hi Peter,

On 28 August 2018 at 08:33, Peter Chen  wrote:
>
>> Add compatibility for extcon-usb-gpio which can handle more than one cable 
>> per
>> instance, allowing coherency of USB cable states (USB/USB-HOST). These states
>> can be generated from ID or/and VBUS pins.
>>
>> In case only one extcon device is associated to the USB device, and this 
>> device
>> supports USB and USB-HOST cable states, we now use it for both VBUS (USB) and
>> ID (USB-HOST) notifier.
>>
>> Signed-off-by: Loic Poulain 
>> ---
>>  v2: no change
>>
>>  drivers/usb/chipidea/core.c | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index
>> cdac778..afe85e2 100644
>> --- a/drivers/usb/chipidea/core.c
>> +++ b/drivers/usb/chipidea/core.c
>> @@ -702,6 +702,17 @@ static int ci_get_platdata(struct device *dev,
>>   ext_id = extcon_get_edev_by_phandle(dev, 1);
>>   if (IS_ERR(ext_id) && PTR_ERR(ext_id) != -ENODEV)
>>   return PTR_ERR(ext_id);
>> +
>> + /*
>> +  * Some extcon devices like extcon-usb-gpio have only one
>> +  * instance for both USB and USB-HOST cable states.
>> +  */
>> + if (!IS_ERR(ext_vbus) && IS_ERR(ext_id)) {
>> + if (extcon_get_state(ext_vbus, EXTCON_USB) >= 0 &&
>> + extcon_get_state(ext_vbus, EXTCON_USB_HOST) >= 0) {
>> + ext_id = ext_vbus;
>> + }
>> + }
>>   }
>>
>
> Hi Loic,
>
> For your case, I suggest changing dts instead of changing source code, you 
> would have both
> vbus and id phandle at your controller dts, and both point to the same extcon 
> device.

Good point, will do.

Regards,
Loic


Re: [Bug 200917] 4.18 regression: I/O error on external icybox disk enclosures

2018-08-29 Thread Klaus Kusche


Hello,

On 24/08/2018 19:28, Alan Stern wrote:

On Fri, 24 Aug 2018, Klaus Kusche wrote:

On 24/08/2018 17:39, Alan Stern wrote:

On Fri, 24 Aug 2018, Klaus Kusche wrote:

On 24/08/2018 16:15, Alan Stern wrote:

On Fri, 24 Aug 2018, Klaus Kusche wrote:

I entered the following USB bug into kernel bugzilla yesterday:

https://bugzilla.kernel.org/show_bug.cgi?id=200917

"Since 4.18, all my external USB3-to-SATA Icybox disk enclosures with usb Id
357d:7788 (seems to be a very common controller chip: Sharkoon QuickPort XT)
fail with the following error when mounting an ext4 fs:
print_req_error: critical target error, dev sdd, sector 2048
Buffer I/O error on dev sdd1, logical block 0, lost sync page write
EXT4-fs (sdd1): I/O error while writing superblock
EXT4-fs (sdd1): mount failed

- They worked before 4.18.
- Reading is definitely ok, async writing seems to work, too.
- The problem occurs with several different disks (I only tested HGST drives).
- The same disks work in enclosures with other controllers."


It does sound like a bug in the enclosure.


It is that specific controller chip.
I tried 3 enclosures with that usb id (all fail since 4.18),
and 5 enclosures with different usb id's (all still work).


Please provide the output from "dmesg".



[ 3692.559336] sd 7:0:0:0: [sdd] 976773168 512-byte logical blocks: (500 GB/466
GiB)
[ 3692.559595] sd 7:0:0:0: [sdd] Write Protect is off
[ 3692.559598] sd 7:0:0:0: [sdd] Mode Sense: 47 00 10 08
[ 3692.559881] sd 7:0:0:0: [sdd] Write cache: enabled, read cache: enabled,
supports DPO and FUA
[ 3692.575820]  sdd: sdd1
[ 3692.576941] sd 7:0:0:0: [sdd] Attached SCSI disk
[ 3725.164065] sd 7:0:0:0: [sdd] tag#0 FAILED Result: hostbyte=DID_OK
driverbyte=DRIVER_SENSE
[ 3725.164071] sd 7:0:0:0: [sdd] tag#0 Sense Key : Illegal Request [current]
[ 3725.164075] sd 7:0:0:0: [sdd] tag#0 Add. Sense: Invalid field in cdb
[ 3725.164080] sd 7:0:0:0: [sdd] tag#0 CDB: Write(10) 2a 08 00 00 08 00 00 00 
08 00


This indicates the error occurred shortly after the drive was plugged
in.  A usbmon trace might be helpful.  Can you collect and send a trace
for bus 4, starting shortly before you plug in the drive and ending
after the error occurs?


cat /sys/kernel/debug/usb/usbmon/4u >usb4.out



Found time to build a kernel with debugfs and usbmon and to test.
Result attached.
Does it contain the command causing the error?


The error does not happen when plugging the drive in.
It happens when rw-mounting an ext4 fs on the drive
(as far as I know, the affected sector 2048 is indeed the ext4 superblock).
Ro-mounting and reading the same ext4 fs in the same enclusoure works fine,
and a vfat on such a drive can even be rw-mounted and successfully written.
Hence, obviously rw-mounting an ext4 fs emits some special write command
which fails with that controller since 4.18.


The command which actually failed was a perfectly standard WRITE(10),
although it has the FUA (Force Unit Access) bit turned on.  Perhaps
that caused the problem, or perhaps an earlier command sent the
controller chip into some sort of error state.


If it's that command: What added that command in 4.18?


--
Prof. Dr. Klaus Kusche
Private address: Rosenberg 41, 07546 Gera, Germany
+49 365 20413058 klaus.kus...@computerix.info https://www.computerix.info
Office address: DHGE Gera, Weg der Freundschaft 4, 07546 Gera, Germany
+49 365 4341 306 klaus.kus...@dhge.de https://www.dhge.de
9d00ac9df9c0 128817022 S Ci:4:001:0 s a3 00  0001 0004 4 <
9d00ac9df9c0 128817058 C Ci:4:001:0 0 4 = 03020100
9d00ac9df9c0 128817069 S Co:4:001:0 s 23 01 0010 0001  0
9d00ac9df9c0 128817081 C Co:4:001:0 0 0
9d00ac9df9c0 128817087 S Ci:4:001:0 s a3 00  0002 0004 4 <
9d00ac9df9c0 128817097 C Ci:4:001:0 0 4 = a002
9d00ac9df9c0 128817101 S Ci:4:001:0 s a3 00  0003 0004 4 <
9d00ac9df9c0 128817110 C Ci:4:001:0 0 4 = a002
9d00ac9df9c0 128817117 S Ci:4:001:0 s a3 00  0004 0004 4 <
9d00ac9df9c0 128817127 C Ci:4:001:0 0 4 = a002
9d00c7d2acc0 128923407 S Ii:4:001:1 -115:2048 4 <
9d00ac9df9c0 128923432 S Ci:4:001:0 s a3 00  0001 0004 4 <
9d00ac9df9c0 128923451 C Ci:4:001:0 0 4 = 0302
9d00ac9df9c0 128923558 S Ci:4:001:0 s a3 00  0001 0004 4 <
9d00ac9df9c0 128923570 C Ci:4:001:0 0 4 = 0302
9d00ac9df9c0 128923573 S Co:4:001:0 s 23 03 0004 0001  0
9d00ac9df9c0 128923583 C Co:4:001:0 0 0
9d00c7d2acc0 128923619 C Ii:4:001:1 0:2048 1 = 02
9d00c7d2acc0 128923624 S Ii:4:001:1 -115:2048 4 <
9d00ac9df9c0 128990074 S Ci:4:001:0 s a3 00  0001 0004 4 <
9d00ac9df9c0 128990101 C Ci:4:001:0 0 4 = 03023000
9d00ac9df9c0 128990107 S Co:4:001:0 s 23 01 0014 0001  0
9d00ac9df9c0 128990116 C Co:4:001:0 0 0
9d00ac9df9c0 128990119 S Co:4:001:0 s 23 01 001d 0001  0
9d00ac9df9c0 128990125 C Co:4:001:0 0 0
9d00ac9df9c0 128990127 S Co:4:001:0 s 23 01 0019 0001  0
9d00ac9df9c0 128990134 C Co:4:001:0 0 0

[PATCH 0/9] usb: dwc2: device: Add service interval support

2018-08-29 Thread Grigor Tovmasyan
This patch set  adds Service Interval support for device mode.

When this mode is enabled core is able to send data any u(f) in current
service interval.

Also in this mode core is able to accept L1 tokens for ISOC IN endpoints.

Reference clock was added in the core to track SOF number internally.
Because of some inaccuracies of reference clock new interrupt was added
to initiate remote wake up and keep sync with the host frame number.

The new interrupt register were added GINTSTS2 for that interrupt.
 


Grigor Tovmasyan (9):
  usb: dwc2: Update registers definitions to support service interval
  usb: dwc2: Add core parameter for service interval support
  usb: dwc2: Add dwc2_gadget_dec_frame_num_by_one() function
  usb: dwc2: Update target (u)frame calculation
  usb: dwc2: Add definitions for new registers
  usb: dwc2: gadget: Add parameters for GREFCLK register
  usb: dwc2: gadget: Program GREFCLK register
  usb: dwc2: gadget: enable WKUP_ALERT interrupt
  usb: dwc2: gadget: Add handler for WkupAlert interrupt

 drivers/usb/dwc2/core.h| 29 +++
 drivers/usb/dwc2/debugfs.c |  1 +
 drivers/usb/dwc2/gadget.c  | 91 ++
 drivers/usb/dwc2/hw.h  | 15 
 drivers/usb/dwc2/params.c  |  6 +++
 5 files changed, 142 insertions(+)

-- 
2.11.0



[PATCH 1/9] usb: dwc2: Update registers definitions to support service interval

2018-08-29 Thread Grigor Tovmasyan
Added GHWCFG4_SERVICE_INTERVAL_SUPPORTED and
DCTL_SERVICE_INTERVAL_SUPPORTED bits definitions to support
service interval based scheduling.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/hw.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 0ca8e7bc7aaf..524629428439 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -312,6 +312,7 @@
 #define GHWCFG4_UTMI_PHY_DATA_WIDTH_SHIFT  14
 #define GHWCFG4_ACG_SUPPORTED  BIT(12)
 #define GHWCFG4_IPG_ISOC_SUPPORTED BIT(11)
+#define GHWCFG4_SERVICE_INTERVAL_SUPPORTED  BIT(10)
 #define GHWCFG4_UTMI_PHY_DATA_WIDTH_8  0
 #define GHWCFG4_UTMI_PHY_DATA_WIDTH_16 1
 #define GHWCFG4_UTMI_PHY_DATA_WIDTH_8_OR_162
@@ -443,6 +444,7 @@
 #define DCFG_DEVSPD_FS48   3
 
 #define DCTL   HSOTG_REG(0x804)
+#define DCTL_SERVICE_INTERVAL_SUPPORTED BIT(19)
 #define DCTL_PWRONPRGDONE  BIT(11)
 #define DCTL_CGOUTNAK  BIT(10)
 #define DCTL_SGOUTNAK  BIT(9)
-- 
2.11.0



[PATCH 2/9] usb: dwc2: Add core parameter for service interval support

2018-08-29 Thread Grigor Tovmasyan
Added core parameter for service interval based scheduling.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/core.h| 9 +
 drivers/usb/dwc2/debugfs.c | 1 +
 drivers/usb/dwc2/gadget.c  | 4 
 drivers/usb/dwc2/params.c  | 4 
 4 files changed, 18 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index cc9c93affa14..2678dc9d559b 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -416,6 +416,9 @@ enum dwc2_ep0_state {
  *  back to DWC2_SPEED_PARAM_HIGH while device is gone.
  * 0 - No (default)
  * 1 - Yes
+ * @service_interval:   Enable service interval based scheduling.
+ *  0 - No
+ *  1 - Yes
  *
  * The following parameters may be specified when starting the module. These
  * parameters define how the DWC_otg controller should be configured. A
@@ -461,6 +464,7 @@ struct dwc2_core_params {
bool lpm_clock_gating;
bool besl;
bool hird_threshold_en;
+   bool service_interval;
u8 hird_threshold;
bool activate_stm_fs_transceiver;
bool ipg_isoc_en;
@@ -605,6 +609,10 @@ struct dwc2_core_params {
  * FIFO sizing is enabled 16 to 32768
  * Actual maximum value is autodetected and also
  * the default.
+ * @service_interval_mode: For enabling service interval based scheduling in 
the
+ * controller.
+ *   0 - Disable
+ *   1 - Enable
  */
 struct dwc2_hw_params {
unsigned op_mode:3;
@@ -635,6 +643,7 @@ struct dwc2_hw_params {
unsigned utmi_phy_data_width:2;
unsigned lpm_mode:1;
unsigned ipg_isoc_en:1;
+   unsigned service_interval_mode:1;
u32 snpsid;
u32 dev_ep_dirs;
u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
diff --git a/drivers/usb/dwc2/debugfs.c b/drivers/usb/dwc2/debugfs.c
index 22d015b0424f..7f62f4cdc265 100644
--- a/drivers/usb/dwc2/debugfs.c
+++ b/drivers/usb/dwc2/debugfs.c
@@ -701,6 +701,7 @@ static int params_show(struct seq_file *seq, void *v)
print_param(seq, p, besl);
print_param(seq, p, hird_threshold_en);
print_param(seq, p, hird_threshold);
+   print_param(seq, p, service_interval);
print_param(seq, p, host_dma);
print_param(seq, p, g_dma);
print_param(seq, p, g_dma_desc);
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 887bea99dce8..a3129f1e87a7 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3312,6 +3312,10 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
dwc2_set_bit(hsotg, DIEPMSK, DIEPMSK_BNAININTRMSK);
}
 
+   /* Enable Service Interval mode if supported */
+   if (using_desc_dma(hsotg) && hsotg->params.service_interval)
+   dwc2_set_bit(hsotg, DCTL, DCTL_SERVICE_INTERVAL_SUPPORTED);
+
dwc2_writel(hsotg, 0, DAINTMSK);
 
dev_dbg(hsotg->dev, "EP0: DIEPCTL0=0x%08x, DOEPCTL0=0x%08x\n",
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index bf7052e037d6..dd3c10d537e2 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -299,6 +299,7 @@ static void dwc2_set_default_params(struct dwc2_hsotg 
*hsotg)
p->hird_threshold_en = true;
p->hird_threshold = 4;
p->ipg_isoc_en = false;
+   p->service_interval = false;
p->max_packet_count = hw->max_packet_count;
p->max_transfer_size = hw->max_transfer_size;
p->ahbcfg = GAHBCFG_HBSTLEN_INCR << GAHBCFG_HBSTLEN_SHIFT;
@@ -592,6 +593,7 @@ static void dwc2_check_params(struct dwc2_hsotg *hsotg)
CHECK_BOOL(besl, (hsotg->hw_params.snpsid >= DWC2_CORE_REV_3_00a));
CHECK_BOOL(hird_threshold_en, hsotg->params.lpm);
CHECK_RANGE(hird_threshold, 0, hsotg->params.besl ? 12 : 7, 0);
+   CHECK_BOOL(service_interval, hw->service_interval_mode);
CHECK_RANGE(max_packet_count,
15, hw->max_packet_count,
hw->max_packet_count);
@@ -780,6 +782,8 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
  GHWCFG4_UTMI_PHY_DATA_WIDTH_SHIFT;
hw->acg_enable = !!(hwcfg4 & GHWCFG4_ACG_SUPPORTED);
hw->ipg_isoc_en = !!(hwcfg4 & GHWCFG4_IPG_ISOC_SUPPORTED);
+   hw->service_interval_mode = !!(hwcfg4 &
+  GHWCFG4_SERVICE_INTERVAL_SUPPORTED);
 
/* fifo sizes */
hw->rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >>
-- 
2.11.0



[PATCH 3/9] usb: dwc2: Add dwc2_gadget_dec_frame_num_by_one() function

2018-08-29 Thread Grigor Tovmasyan
Added dwc2_gadget_dec_frame_num_by_one() function in gadget.c.
This function will be used to calculate descriptor frame number field
value. For service interval mode frame number in descriptor should point
to last (u)frame in the interval.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/gadget.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index a3129f1e87a7..58903b5c22a4 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -122,6 +122,24 @@ static inline void dwc2_gadget_incr_frame_num(struct 
dwc2_hsotg_ep *hs_ep)
}
 }
 
+/**
+ * dwc2_gadget_dec_frame_num_by_one - Decrements the targeted frame number
+ *by one.
+ * @hs_ep: The endpoint.
+ *
+ * This function used in service interval based scheduling flow to calculate
+ * descriptor frame number filed value. For service interval mode frame
+ * number in descriptor should point to last (u)frame in the interval.
+ *
+ */
+static inline void dwc2_gadget_dec_frame_num_by_one(struct dwc2_hsotg_ep 
*hs_ep)
+{
+   if (hs_ep->target_frame)
+   hs_ep->target_frame -= 1;
+   else
+   hs_ep->target_frame = DSTS_SOFFN_LIMIT;
+}
+
 /**
  * dwc2_hsotg_en_gsint - enable one or more of the general interrupt
  * @hsotg: The device state
-- 
2.11.0



[PATCH 5/9] usb: dwc2: Add definitions for new registers

2018-08-29 Thread Grigor Tovmasyan
New registers were added to dwc otg core.

GREFCLK - This register used to control ref_clk parameters.

GINTSTS2 - New WKUP_ALERT interrupt was added.

GINTMSK2 - Mask register for GINTSTS2.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/hw.h | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h
index 524629428439..2b1ea441b7d4 100644
--- a/drivers/usb/dwc2/hw.h
+++ b/drivers/usb/dwc2/hw.h
@@ -405,6 +405,19 @@
 #define ADPCTL_PRB_DSCHRG_MASK (0x3 << 0)
 #define ADPCTL_PRB_DSCHRG_SHIFT0
 
+#define GREFCLKHSOTG_REG(0x0064)
+#define GREFCLK_REFCLKPER_MASK (0x1 << 15)
+#define GREFCLK_REFCLKPER_SHIFT15
+#define GREFCLK_REF_CLK_MODE   BIT(14)
+#define GREFCLK_SOF_CNT_WKUP_ALERT_MASK(0x3ff)
+#define GREFCLK_SOF_CNT_WKUP_ALERT_SHIFT0
+
+#define GINTMSK2   HSOTG_REG(0x0068)
+#define GINTMSK2_WKUP_ALERT_INT_MSKBIT(0)
+
+#define GINTSTS2   HSOTG_REG(0x006c)
+#define GINTSTS2_WKUP_ALERT_INTBIT(0)
+
 #define HPTXFSIZ   HSOTG_REG(0x100)
 /* Use FIFOSIZE_* constants to access this register */
 
-- 
2.11.0



[PATCH 4/9] usb: dwc2: Update target (u)frame calculation

2018-08-29 Thread Grigor Tovmasyan
In service interval based scheduling target (u)frame must be
set as a last frame in this the service interval.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/gadget.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 58903b5c22a4..004701cad25c 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2830,6 +2830,23 @@ static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep 
*hs_ep)
if (using_desc_dma(hsotg)) {
hs_ep->target_frame = hsotg->frame_number;
dwc2_gadget_incr_frame_num(hs_ep);
+
+   /* In service interval mode target_frame must
+* be set to last (u)frame of the service interval.
+*/
+   if (hsotg->params.service_interval) {
+   /* Set target_frame to the first (u)frame of
+* the service interval
+*/
+   hs_ep->target_frame &= ~hs_ep->interval + 1;
+
+   /* Set target_frame to the last (u)frame of
+* the service interval
+*/
+   dwc2_gadget_incr_frame_num(hs_ep);
+   dwc2_gadget_dec_frame_num_by_one(hs_ep);
+   }
+
dwc2_gadget_start_isoc_ddma(hs_ep);
return;
}
-- 
2.11.0



[PATCH 6/9] usb: dwc2: gadget: Add parameters for GREFCLK register

2018-08-29 Thread Grigor Tovmasyan
Added ref_clk_per and sof_cnt_wkup_alert parameters in
dwc2_core_params struct and set default values.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/core.h   | 18 ++
 drivers/usb/dwc2/params.c |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2678dc9d559b..655f5274e801 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -393,6 +393,20 @@ enum dwc2_ep0_state {
  * 0 - No
  * 1 - Yes
  * @hird_threshold:Value of BESL or HIRD Threshold.
+ * @ref_clk_per:Indicates in terms of pico seconds the period
+ *  of ref_clk.
+ * 62500 - 16MHz
+ *  58823 - 17MHz
+ *  52083 - 19.2MHz
+ * 5 - 20MHz
+ * 41666 - 24MHz
+ * 3 - 30MHz (default)
+ * 25000 - 40MHz
+ * @sof_cnt_wkup_alert: Indicates in term of number of SOF's after which
+ *  the controller should generate an interrupt if the
+ *  device had been in L1 state until that period.
+ *  This is used by SW to initiate Remote WakeUp in the
+ *  controller so as to sync to the uF number from the 
host.
  * @activate_stm_fs_transceiver: Activate internal transceiver using GGPIO
  * register.
  * 0 - Deactivate the transceiver (default)
@@ -472,6 +486,10 @@ struct dwc2_core_params {
u32 max_transfer_size;
u32 ahbcfg;
 
+   /* GREFCLK parameters */
+   u32 ref_clk_per;
+   u16 sof_cnt_wkup_alert;
+
/* Host parameters */
bool host_dma;
bool dma_desc_enable;
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index dd3c10d537e2..d150984406ee 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -303,6 +303,8 @@ static void dwc2_set_default_params(struct dwc2_hsotg 
*hsotg)
p->max_packet_count = hw->max_packet_count;
p->max_transfer_size = hw->max_transfer_size;
p->ahbcfg = GAHBCFG_HBSTLEN_INCR << GAHBCFG_HBSTLEN_SHIFT;
+   p->ref_clk_per = 3;
+   p->sof_cnt_wkup_alert = 100;
 
if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
(hsotg->dr_mode == USB_DR_MODE_OTG)) {
-- 
2.11.0



[PATCH 7/9] usb: dwc2: gadget: Program GREFCLK register

2018-08-29 Thread Grigor Tovmasyan
Added dwc2_gadget_program_ref_clk function to program GREFCLK
register in device mode.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/core.h   |  2 ++
 drivers/usb/dwc2/gadget.c | 23 +++
 2 files changed, 25 insertions(+)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 655f5274e801..30bab8463c96 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -1381,6 +1381,7 @@ int dwc2_hsotg_tx_fifo_count(struct dwc2_hsotg *hsotg);
 int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg);
 int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
 void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg);
+void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg);
 #else
 static inline int dwc2_hsotg_remove(struct dwc2_hsotg *dwc2)
 { return 0; }
@@ -1415,6 +1416,7 @@ static inline int dwc2_hsotg_tx_fifo_total_depth(struct 
dwc2_hsotg *hsotg)
 static inline int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg)
 { return 0; }
 static inline void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg) {}
+static inline void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg) {}
 #endif
 
 #if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 004701cad25c..66401ffeb5a2 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3407,6 +3407,10 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg 
*hsotg,
/* configure the core to support LPM */
dwc2_gadget_init_lpm(hsotg);
 
+   /* program GREFCLK register if needed */
+   if (using_desc_dma(hsotg) && hsotg->params.service_interval)
+   dwc2_gadget_program_ref_clk(hsotg);
+
/* must be at-least 3ms to allow bus to see disconnect */
mdelay(3);
 
@@ -4985,6 +4989,25 @@ void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg)
dev_dbg(hsotg->dev, "GLPMCFG=0x%08x\n", dwc2_readl(hsotg, GLPMCFG));
 }
 
+/**
+ * dwc2_gadget_program_ref_clk - Program GREFCLK register in device mode
+ *
+ * @hsotg: Programming view of DWC_otg controller
+ *
+ */
+void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg)
+{
+   u32 val = 0;
+
+   val |= GREFCLK_REF_CLK_MODE;
+   val |= hsotg->params.ref_clk_per << GREFCLK_REFCLKPER_SHIFT;
+   val |= hsotg->params.sof_cnt_wkup_alert <<
+  GREFCLK_SOF_CNT_WKUP_ALERT_SHIFT;
+
+   dwc2_writel(hsotg, val, GREFCLK);
+   dev_dbg(hsotg->dev, "GREFCLK=0x%08x\n", dwc2_readl(hsotg, GREFCLK));
+}
+
 /**
  * dwc2_gadget_enter_hibernation() - Put controller in Hibernation.
  *
-- 
2.11.0



[PATCH 8/9] usb: dwc2: gadget: enable WKUP_ALERT interrupt

2018-08-29 Thread Grigor Tovmasyan
WKUP_ALERT interrupt should be unmask when lpm mode is enabled.

This interrupt is asserted when the device is in L1 for the duration
mentioned in GREFCLK.SOF_CNN_WKUP_ALERT. This is used to alert SW to
initiate Remote wake up so that the device resumes in time in order not
to lose sync with the host frame number.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/gadget.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 66401ffeb5a2..4f99f2e44a5b 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4987,6 +4987,10 @@ void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg)
val |= hsotg->params.besl ? GLPMCFG_ENBESL : 0;
dwc2_writel(hsotg, val, GLPMCFG);
dev_dbg(hsotg->dev, "GLPMCFG=0x%08x\n", dwc2_readl(hsotg, GLPMCFG));
+
+   /* Unmask WKUP_ALERT Interrupt */
+   if (hsotg->params.service_interval)
+   dwc2_set_bit(hsotg, GINTMSK2, GINTMSK2_WKUP_ALERT_INT_MSK);
 }
 
 /**
-- 
2.11.0



[PATCH 9/9] usb: dwc2: gadget: Add handler for WkupAlert interrupt

2018-08-29 Thread Grigor Tovmasyan
Added interrupt handler for WkupAlert interrupt.

This interrupt should initiate Remote Wake up.

Signed-off-by: Grigor Tovmasyan 
---
 drivers/usb/dwc2/gadget.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 4f99f2e44a5b..8c558a1b7add 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -245,6 +245,27 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg 
*hsotg)
return tx_addr_max - addr;
 }
 
+/**
+ * dwc2_gadget_wkup_alert_handler - Handler for WKUP_ALERT interrupt
+ *
+ * @hsotg: Programming view of the DWC_otg controller
+ *
+ */
+static void dwc2_gadget_wkup_alert_handler(struct dwc2_hsotg *hsotg)
+{
+   u32 gintsts2;
+   u32 gintmsk2;
+
+   gintsts2 = dwc2_readl(hsotg, GINTSTS2);
+   gintmsk2 = dwc2_readl(hsotg, GINTMSK2);
+
+   if (gintsts2 & GINTSTS2_WKUP_ALERT_INT) {
+   dev_dbg(hsotg->dev, "%s: Wkup_Alert_Int\n", __func__);
+   dwc2_clear_bit(hsotg, GINTSTS2, GINTSTS2_WKUP_ALERT_INT);
+   dwc2_set_bit(hsotg, DCFG, DCTL_RMTWKUPSIG);
+   }
+}
+
 /**
  * dwc2_hsotg_tx_fifo_average_depth - returns average depth of device mode
  * TX FIFOs
@@ -3719,6 +3740,10 @@ static irqreturn_t dwc2_hsotg_irq(int irq, void *pw)
if (gintsts & IRQ_RETRY_MASK && --retry_count > 0)
goto irq_retry;
 
+   /* Check WKUP_ALERT interrupt*/
+   if (hsotg->params.service_interval)
+   dwc2_gadget_wkup_alert_handler(hsotg);
+
spin_unlock(&hsotg->lock);
 
return IRQ_HANDLED;
-- 
2.11.0



[PATCH] usb: gadget: atmel: remove pointless retrieval of DT name property

2018-08-29 Thread Rob Herring
The name is always non-NULL and then is not used anywhere in this function,
so remove it.

Cc: Nicolas Ferre 
Cc: Felipe Balbi 
Cc: Greg Kroah-Hartman 
Cc: Alexandre Belloni 
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-usb@vger.kernel.org
Signed-off-by: Rob Herring 
---
 drivers/usb/gadget/udc/atmel_usba_udc.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c 
b/drivers/usb/gadget/udc/atmel_usba_udc.c
index 17147b8c771e..5729935c5eb2 100644
--- a/drivers/usb/gadget/udc/atmel_usba_udc.c
+++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
@@ -2004,7 +2004,6 @@ static struct usba_ep * atmel_udc_of_init(struct 
platform_device *pdev,
struct usba_udc *udc)
 {
u32 val;
-   const char *name;
struct device_node *np = pdev->dev.of_node;
const struct of_device_id *match;
struct device_node *pp;
@@ -2094,11 +2093,6 @@ static struct usba_ep * atmel_udc_of_init(struct 
platform_device *pdev,
ep->can_dma = of_property_read_bool(pp, "atmel,can-dma");
ep->can_isoc = of_property_read_bool(pp, "atmel,can-isoc");
 
-   ret = of_property_read_string(pp, "name", &name);
-   if (ret) {
-   dev_err(&pdev->dev, "of_probe: name error(%d)\n", ret);
-   goto err;
-   }
sprintf(ep->name, "ep%d", ep->index);
ep->ep.name = ep->name;
 
-- 
2.17.1



RE: [PATCH v2 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

2018-08-29 Thread Ajay Gupta
Hi Andy,

> > Latest NVIDIA GPU card has USB Type-C interface. There is a
> > Type-C controller which can be accessed over I2C.
> >
> > This driver add I2C bus driver to communicate with Type-C controller.
> > I2C client driver will be part of USB Type-C UCSI driver.
> 
> >  drivers/i2c/busses/i2c-gpu.c | 493
> +++
> 
> Can we got more better name, which includes vendor and/or model of the I2C
> host?
Sure will change to i2c-nvidia-gpu.c

> > +/* STATUS definitions  */
> > +#define STATUS_SUCCESS 0
> > +#define STATUS_UNSUCCESSFUL0x8000UL
> > +#define STATUS_TIMEOUT 0x8001UL
> > +#define STATUS_IO_DEVICE_ERROR 0x8002UL
> > +#define STATUS_IO_TIMEOUT  0x8004UL
> > +#define STATUS_IO_PREEMPTED0x8008UL
> 
> Looks slightly different from my point of view, something like
> 
> /* Bit 31 shows error condition while LSB encodes the error code */
> STATUS_TIMEOUT BIT(0)
> ...
> STATUS_ERROR  BIT(31)
Will fix.

> > +   dev_dbg(dev, "%s: %p (I2C_MST_HYBRID_PADCTL) <- %08x", __func__,
> > +   (gdev->regs + I2C_MST_HYBRID_PADCTL), val);
> 
> Parens are redundant, __func__ is redundant.
Will fix.

> > +   dev_dbg(dev, "%s: %p (I2C_MST_I2C0_TIMING) <- %08x", __func__,
> > +   gdev->regs + I2C_MST_I2C0_TIMING, val);
> 
> Ditto. Check your debug messages, and perheps even drop some.
Will fix.

> > +static u32 i2c_check_status(struct gpu_i2c_dev *gdev)
> > +{
> 
> > +   while (time_is_after_jiffies(target)) {
> > +   }
> 
> For functions like this better to get in a form
> do {
> } while().
Ok, will fix.

> There is no guarantee that it runs even once in your case.
> 
> > +   dev_err(dev, "%si2c timeout", __func__);
> 
> No space?
Ok, will fix.
> 
> > +   val = readl(gdev->regs + I2C_MST_DATA);
> > +   switch (len) {
> > +   case 1:
> > +   data[0] = (val >> 0) & 0xff;
> > +   break;
> > +   case 2:
> > +   data[0] = (val >> 8) & 0xff;
> > +   data[1] = (val >> 0) & 0xff;
> > +   break;
> > +   case 3:
> > +   data[0] = (val >> 16) & 0xff;
> > +   data[1] = (val >> 8) & 0xff;
> > +   data[2] = (val >> 0) & 0xff;
> > +   break;
> > +   case 4:
> > +   data[0] = (val >> 24) & 0xff;
> > +   data[1] = (val >> 16) & 0xff;
> > +   data[2] = (val >> 8) & 0xff;
> > +   data[3] = (val >> 0) & 0xff;
> > +   break;
> 
> Redundant  & 0xff.
> We have get_unaligned*(), put_unaligned*() and many variations of
> cpu_to_Xe*() and Xe*_to_cpu().
Ok, will fix.
> 
> > +   u32 val = 0;
> 
> Redundant assignment.
Ok, will fix.
> 
> > +   val = addr << I2C_MST_ADDR_DAB;
> 
> > +   val = 0;
> 
> Ditto. What's wrong with assign value below directly?
> 
> > +   val |= I2C_MST_CNTL_GEN_START | I2C_MST_CNTL_CMD_NONE |
> > +   I2C_MST_CNTL_GEN_NACK;
> 
> > +   u32 val = 0;
> 
> Check your code for these kind of style mistakes.
> 
> > +/* gdev i2c adapter */
> 
> Pointless.
> 
> > +static int gpu_i2c_master_xfer(struct i2c_adapter *adap,
> > +   struct i2c_msg *msgs, int num)
> > +{
> > +   struct gpu_i2c_dev *gdev = i2c_get_adapdata(adap);
> > +   struct device *dev = &gdev->pci_dev->dev;
> > +   int retry1b = 10;
> > +   u32 status;
> > +   int i, j;
> 
> > +   goto exit;
> > +exit_stop:
> > +   status = i2c_manual_stop(gdev);
> > +   if (status != STATUS_SUCCESS)
> > +   dev_err(dev, "i2c_manual_stop failed %x", status);
> > +exit:
> > +   mutex_unlock(&gdev->mutex);
> > +   return i;
> > +}
> 
> Ouch! Besides many small style issues and redundancy (like __LINE__),
> this function needs to be refactored to few smaller and readable ones.
Ok, will fix.
> 
> > +#define PCI_CLASS_SERIAL_UNKNOWN   0x0c80
> 
> > +/* pci driver */
> 
> Pointless.
Ok, will fix.
> 
> > +static const struct pci_device_id gpu_i2c_ids[] = {
> > +   { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> > +   PCI_CLASS_SERIAL_UNKNOWN << 8, 0xff00},
> 
> Are you sure?!
Yes, we want to identify using vendor ID and class code. 
Currently there is no class code defined for UCSI device over PCI so using 
UNKNOWN.

> 
> > +   { },
> 
> Terminator line better w/o comma.
Ok, will fix.
> 
> > +};
> > +MODULE_DEVICE_TABLE(pci, gpu_i2c_ids);
> 
> > +static int gpu_i2c_probe(struct pci_dev *dev, const struct pci_device_id
> *id)
> > +{
> > +   struct gpu_i2c_dev *gdev;
> > +   int status;
> > +
> 
> > +   dev_info(&dev->dev,
> > +   "dev %p id %08x %08x sub %08x %08x class %08x %08x\n",
> > +   dev, id->vendor, id->device, id->subvendor, id->subdevice,
> > +   id->class, id->class_mask);
> 
> Useless. We have PCI core printed this information out sever

RE: [v2,1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

2018-08-29 Thread Ajay Gupta
Hi Thierry,

> > Latest NVIDIA GPU card has USB Type-C interface. There is a Type-C
> > controller which can be accessed over I2C.
> >
> > This driver add I2C bus driver to communicate with Type-C controller.
> > I2C client driver will be part of USB Type-C UCSI driver.
> >
> > Signed-off-by: Ajay Gupta 
> > ---
> > Changes from v1 -> v2: None
> >
> >  Documentation/i2c/busses/i2c-gpu |  18 ++
> >  MAINTAINERS  |   7 +
> >  drivers/i2c/busses/Kconfig   |   9 +
> >  drivers/i2c/busses/Makefile  |   1 +
> >  drivers/i2c/busses/i2c-gpu.c | 493
> +++
> >  5 files changed, 528 insertions(+)
> >  create mode 100644 Documentation/i2c/busses/i2c-gpu  create mode
> > 100644 drivers/i2c/busses/i2c-gpu.c
> 
> Hi Ajay,
> 
> I think this looks pretty good. A couple of minor, mostly nit-picky, comments
> below.
> 
> >
> > diff --git a/Documentation/i2c/busses/i2c-gpu
> > b/Documentation/i2c/busses/i2c-gpu
> > new file mode 100644
> > index 000..873ba34
> > --- /dev/null
> > +++ b/Documentation/i2c/busses/i2c-gpu
> 
> I think this is too generic. Maybe use something like i2c-nvidia-gpu here and
> everywhere else, to make it explicit that this is for NVIDIA GPUs rather than
> GPUs in general.
ok

> > @@ -0,0 +1,18 @@
> > +Kernel driver i2c-gpu
> > +
> > +Datasheet: not publicly available.
> > +
> > +Authors:
> > +   Ajay Gupta 
> > +
> > +Description
> > +---
> > +
> > +i2c-gpu is a driver for I2C controller included in NVIDIA Turing and
> > +later GPUs and it is used to communicate with Type-C controller on GPUs.
> > +
> > +If your 'lspci -v' listing shows something like the following,
> > +
> > +01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9
> > +(rev a1)
> > +
> > +then this driver should support the I2C controller of your GPU.
> > diff --git a/MAINTAINERS b/MAINTAINERS index b2fcd1c..e99f8a2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6796,6 +6796,13 @@ L:   linux-a...@vger.kernel.org
> >  S: Maintained
> >  F: drivers/i2c/i2c-core-acpi.c
> >
> > +I2C CONTROLLER DRIVER FOR NVIDIA GPU
> > +M: Ajay Gupta 
> > +L: linux-...@vger.kernel.org
> > +S: Maintained
> > +F: Documentation/i2c/busses/i2c-gpu
> > +F: drivers/i2c/busses/i2c-gpu.c
> > +
> >  I2C MUXES
> >  M: Peter Rosin 
> >  L: linux-...@vger.kernel.org
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 451d4ae..ff8b2d4 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985
> >   This driver can also be built as a module.  If so, the module
> >   will be called i2c-nforce2-s4985.
> >
> > +config I2C_GPU
> > +   tristate "NVIDIA GPU I2C controller"
> > +   depends on PCI
> > +   help
> > + If you say yes to this option, support will be included for the
> > + NVIDIA GPU I2C controller which is used to communicate with the
> GPU's
> > + Type-C controller. This driver can also be built as a module called
> > + i2c-gpu.ko.
> > +
> >  config I2C_SIS5595
> > tristate "SiS 5595"
> > depends on PCI
> > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
> > index 18b26af..15d2894 100644
> > --- a/drivers/i2c/busses/Makefile
> > +++ b/drivers/i2c/busses/Makefile
> > @@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o
> >  obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
> >  obj-$(CONFIG_SCx200_ACB)   += scx200_acb.o
> >  obj-$(CONFIG_I2C_FSI)  += i2c-fsi.o
> > +obj-$(CONFIG_I2C_GPU)  += i2c-gpu.o
> >
> >  ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG diff --git
> > a/drivers/i2c/busses/i2c-gpu.c b/drivers/i2c/busses/i2c-gpu.c new file
> > mode 100644 index 000..0fd2944
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-gpu.c
> > @@ -0,0 +1,493 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Nvidia GPU I2C controller Driver
> > + *
> > + * Copyright (C) 2018 NVIDIA Corporation. All rights reserved.
> > + * Author: Ajay Gupta 
> > + *
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* STATUS definitions  */
> > +#define STATUS_SUCCESS 0
> > +#define STATUS_UNSUCCESSFUL0x8000UL
> > +#define STATUS_TIMEOUT 0x8001UL
> > +#define STATUS_IO_DEVICE_ERROR 0x8002UL
> > +#define STATUS_IO_TIMEOUT  0x8004UL
> > +#define STATUS_IO_PREEMPTED0x8008UL
> 
> This seems a little odd. Can these not be converted to standard errno codes?
> It's more idiomatic to return 0 on success (like you define in the above) and 
> a
> negative error code on failure. If you use that scheme, there's no need to 
> have
> a symbolic name for success because it is used pretty much everywhere.
> 
> See include/uapi/asm-generic/errno{,-base}.h for a list of standard error
> codes. I think 

RE: [PATCH v2 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-08-29 Thread Ajay Gupta
Hi Heikki,

> On Fri, Aug 24, 2018 at 02:33:36PM -0700, Ajay Gupta wrote:
> > Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
> > over I2C interface.
> >
> > This UCSI I2C driver uses I2C bus driver interface for communicating
> > with Type-C controller.
> 
> Cool. The patch looks fairly good to me, but I put a few comments
> below.
Thanks for review.

> 
> > Signed-off-by: Ajay Gupta 
> > ---
> > Changes from v1 -> v2:
> > Fixed identation in drivers/usb/typec/ucsi/Kconfig
> >
> >  drivers/usb/typec/ucsi/Kconfig|  10 +
> >  drivers/usb/typec/ucsi/Makefile   |   2 +
> >  drivers/usb/typec/ucsi/ucsi_i2c_ccg.c | 591
> ++
> 
> CCGx controllers support also SPI and UART AFAIK. Though, I'm not sure
> how commonly they are used (I would expect I2C to be the most common
> with these controllers), the driver should ultimately work with both
> of those busses as well.
> 
> To avoid confusion, and potential driver duplicates in the future,
> just name the driver ucsi_ccg.c for now, and also s/i2c_ccg/ccg/
> everything in the driver (except the i2c_driver structure of course).
Sure.


> >  3 files changed, 603 insertions(+)
> >  create mode 100644 drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
> >
> > diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
> > index e36d6c7..0ce9d48 100644
> > --- a/drivers/usb/typec/ucsi/Kconfig
> > +++ b/drivers/usb/typec/ucsi/Kconfig
> > @@ -23,6 +23,16 @@ config TYPEC_UCSI
> >
> >  if TYPEC_UCSI
> >
> > +config UCSI_I2C_CCG
> > +   tristate "UCSI I2C Interface Driver for Cypress CCGx"
> > +   depends on I2C_GPU
> 
> Why does it need to depend only on your I2C controller?
> 
> I think that should be just "depends on I2C".
ok
 
> > +   help
> > + This driver enables UCSI support on NVIDIA GPUs that expose a
> > + Cypress CCGx Type-C controller over I2C interface.
> > +
> > + To compile the driver as a module, choose M here: the module will
> be
> > + called ucsi_i2c_ccg.ko.
> > +
> >  config UCSI_ACPI
> > tristate "UCSI ACPI Interface Driver"
> > depends on ACPI
> > diff --git a/drivers/usb/typec/ucsi/Makefile
> b/drivers/usb/typec/ucsi/Makefile
> > index 7afbea5..4439482 100644
> > --- a/drivers/usb/typec/ucsi/Makefile
> > +++ b/drivers/usb/typec/ucsi/Makefile
> > @@ -8,3 +8,5 @@ typec_ucsi-y:= ucsi.o
> >  typec_ucsi-$(CONFIG_TRACING)   += trace.o
> >
> >  obj-$(CONFIG_UCSI_ACPI)+= ucsi_acpi.o
> > +
> > +obj-$(CONFIG_UCSI_I2C_CCG) += ucsi_i2c_ccg.o
> > diff --git a/drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
> b/drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
> > new file mode 100644
> > index 000..587e3f8
> > --- /dev/null
> > +++ b/drivers/usb/typec/ucsi/ucsi_i2c_ccg.c
> > @@ -0,0 +1,591 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * UCSI I2C driver for Cypress CCGx Type-C controller
> > + *
> > + * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
> > + * Author: Ajay Gupta 
> > + *
> > + * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include "ucsi.h"
> > +
> > +struct ucsi_i2c_ccg {
> > +   struct device *dev;
> > +   struct ucsi *ucsi;
> > +   struct ucsi_ppm ppm;
> > +   struct i2c_client *client;
> > +   int irq;
> > +   bool wake_enabled;
> > +   unsigned char ver;
> > +};
> > +
> > +#define CCGX_I2C_RAB_DEVICE_MODE   0xU
> > +#define CCGX_I2C_RAB_BOOT_MODE_REASON
>   0x0001U
> > +#define CCGX_I2C_RAB_READ_SILICON_ID   0x0002U
> > +#define CCGX_I2C_RAB_INTR_REG  0x0006U
> > +#define CCGX_I2C_RAB_RESET 0x0008U
> > +#define CCGX_I2C_RAB_READ_ALL_VERSION  0x0010U
> > +#define CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER \
> > +   (CCGX_I2C_RAB_READ_ALL_VERSION + 0x00)
> > +#define CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER_BASE \
> > +   (CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER
> + 0)
> > +#define CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER_FW \
> > +   (CCGX_I2C_RAB_READ_ALL_VERSION_BOOTLOADER
> + 4)
> > +#define CCGX_I2C_RAB_READ_ALL_VERSION_APP \
> > +   (CCGX_I2C_RAB_READ_ALL_VERSION + 0x08)
> > +#define CCGX_I2C_RAB_READ_ALL_VERSION_APP_BASE \
> > +   (CCGX_I2C_RAB_READ_ALL_VERSION_APP + 0)
> > +#define CCGX_I2C_RAB_READ_ALL_VERSION_APP_FW \
> > +   (CCGX_I2C_RAB_READ_ALL_VERSION_APP + 4)
> > +#define CCGX_I2C_RAB_FW2_VERSION   0x0020U
> > +#define CCGX_I2C_RAB_PDPORT_ENABLE 0x002CU
> > +#define CCGX_I2C_RAB_UCSI_STATUS   0x0038U
> > +#define CCGX_I2C_RAB_UCSI_CONTROL  0x0039U
> > +#define CCGX_I2C_RAB_UCSI_CONTROL_STOP 0x2U
> > +#define CCGX_I2C_RAB_UCSI_CONTROL_START0x1U
> > +#define CCGX_I2C_RAB_HPI_VE

[PATCH v3 2/2] usb: typec: ucsi: add support for Cypress CCGx

2018-08-29 Thread ajaykuee
From: Ajay Gupta 

Latest NVIDIA GPU cards have a Cypress CCGx Type-C controller
over I2C interface.

This UCSI I2C driver uses I2C bus driver interface for communicating
with Type-C controller.

Signed-off-by: Ajay Gupta 
---
Changes from v1 -> v2
Fixed identation in drivers/usb/typec/ucsi/Kconfig
Changes from v2 -> v3
Fixed most of comments from Heikki
Rename ucsi_i2c_ccg.c -> ucsi_ccg.c

 drivers/usb/typec/ucsi/Kconfig|  10 +
 drivers/usb/typec/ucsi/Makefile   |   2 +
 drivers/usb/typec/ucsi/ucsi_ccg.c | 417 ++
 3 files changed, 429 insertions(+)
 create mode 100644 drivers/usb/typec/ucsi/ucsi_ccg.c

diff --git a/drivers/usb/typec/ucsi/Kconfig b/drivers/usb/typec/ucsi/Kconfig
index e36d6c7..7811888 100644
--- a/drivers/usb/typec/ucsi/Kconfig
+++ b/drivers/usb/typec/ucsi/Kconfig
@@ -23,6 +23,16 @@ config TYPEC_UCSI
 
 if TYPEC_UCSI
 
+config UCSI_CCG
+   tristate "UCSI Interface Driver for Cypress CCGx"
+   depends on I2C
+   help
+ This driver enables UCSI support on platforms that expose a
+ Cypress CCGx Type-C controller over I2C interface.
+
+ To compile the driver as a module, choose M here: the module will be
+ called ucsi_ccg.
+
 config UCSI_ACPI
tristate "UCSI ACPI Interface Driver"
depends on ACPI
diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index 7afbea5..2f4900b 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -8,3 +8,5 @@ typec_ucsi-y:= ucsi.o
 typec_ucsi-$(CONFIG_TRACING)   += trace.o
 
 obj-$(CONFIG_UCSI_ACPI)+= ucsi_acpi.o
+
+obj-$(CONFIG_UCSI_CCG) += ucsi_ccg.o
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c 
b/drivers/usb/typec/ucsi/ucsi_ccg.c
new file mode 100644
index 000..e69cc9f
--- /dev/null
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -0,0 +1,417 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * UCSI driver for Cypress CCGx Type-C controller
+ *
+ * Copyright (C) 2017-2018 NVIDIA Corporation. All rights reserved.
+ * Author: Ajay Gupta 
+ *
+ * Some code borrowed from drivers/usb/typec/ucsi/ucsi_acpi.c
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "ucsi.h"
+
+struct ucsi_ccg {
+   struct device *dev;
+   struct ucsi *ucsi;
+   struct ucsi_ppm ppm;
+   struct i2c_client *client;
+   int irq;
+};
+
+#define CCGX_I2C_RAB_DEVICE_MODE   0x0
+#define CCGX_I2C_RAB_BOOT_MODE_REASON  0x1
+#define CCGX_I2C_RAB_READ_SILICON_ID   0x2
+#define CCGX_I2C_RAB_INTR_REG  0x6
+#define CCGX_I2C_RAB_FW1_VERSION   0x28
+#define CCGX_I2C_RAB_FW2_VERSION   0x20
+#define CCGX_I2C_RAB_UCSI_CONTROL  0x39
+#define CCGX_I2C_RAB_UCSI_CONTROL_STOP 0x2
+#define CCGX_I2C_RAB_UCSI_CONTROL_START0x1
+#define CCGX_I2C_RAB_RESPONSE_REG  0x7E
+#define CCGX_I2C_RAB_UCSI_DATA_BLOCK   0xf000
+#define CCGX_I2C_RAB_RESPONSE_REG_RESET_COMPLETE   0x80
+
+static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+{
+   struct device *dev = uc->dev;
+   struct i2c_client *client = uc->client;
+   unsigned char buf[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = client->addr,
+   .flags  = 0x0,
+   .len= 0x2,
+   .buf= buf,
+   },
+   {
+   .addr   = client->addr,
+   .flags  = I2C_M_RD,
+   .buf= data,
+   },
+   };
+   u32 rlen, rem_len = len;
+   int err = -EIO;
+
+   while (rem_len > 0) {
+   msgs[1].buf = &data[len - rem_len];
+   rlen = min_t(u16, rem_len, 4);
+   msgs[1].len = rlen;
+   buf[0] = (rab >> 0) & 0xff;
+   buf[1] = (rab >> 8) & 0xff;
+   err = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+   if (err == ARRAY_SIZE(msgs)) {
+   err = 0;
+   } else if (err >= 0) {
+   dev_err(dev, "i2c_transfer failed, err %d\n", err);
+   return -EIO;
+   }
+   rab += rlen;
+   rem_len -= rlen;
+   }
+
+   return err;
+}
+
+static int ccg_write(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
+{
+   struct device *dev = uc->dev;
+   struct i2c_client *client = uc->client;
+   unsigned char buf[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = client->addr,
+   .flags  = 0x0,
+   .len= 0x2,
+   .buf= buf,
+   },
+   {
+ 

[PATCH v3 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

2018-08-29 Thread ajaykuee
From: Ajay Gupta 

Latest NVIDIA GPU card has USB Type-C interface. There is a
Type-C controller which can be accessed over I2C.

This driver adds I2C bus driver to communicate with Type-C controller.
I2C client driver will be part of USB Type-C UCSI driver.

Signed-off-by: Ajay Gupta 
---
Changes from v1 -> v2
None
Changes from v2 -> v3
Fixed review comments from Andy and Thierry
Rename i2c-gpu.c -> i2c-nvidia-gpu.c

 Documentation/i2c/busses/i2c-nvidia-gpu |  18 ++
 MAINTAINERS |   7 +
 drivers/i2c/busses/Kconfig  |   9 +
 drivers/i2c/busses/Makefile |   1 +
 drivers/i2c/busses/i2c-nvidia-gpu.c | 389 
 5 files changed, 424 insertions(+)
 create mode 100644 Documentation/i2c/busses/i2c-nvidia-gpu
 create mode 100644 drivers/i2c/busses/i2c-nvidia-gpu.c

diff --git a/Documentation/i2c/busses/i2c-nvidia-gpu 
b/Documentation/i2c/busses/i2c-nvidia-gpu
new file mode 100644
index 000..31884d2
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-nvidia-gpu
@@ -0,0 +1,18 @@
+Kernel driver i2c-nvidia-gpu
+
+Datasheet: not publicly available.
+
+Authors:
+   Ajay Gupta 
+
+Description
+---
+
+i2c-nvidia-gpu is a driver for I2C controller included in NVIDIA Turing
+and later GPUs and it is used to communicate with Type-C controller on GPUs.
+
+If your 'lspci -v' listing shows something like the following,
+
+01:00.3 Serial bus controller [0c80]: NVIDIA Corporation Device 1ad9 (rev a1)
+
+then this driver should support the I2C controller of your GPU.
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ad052a..2d1c5a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6797,6 +6797,13 @@ L:   linux-a...@vger.kernel.org
 S: Maintained
 F: drivers/i2c/i2c-core-acpi.c
 
+I2C CONTROLLER DRIVER FOR NVIDIA GPU
+M: Ajay Gupta 
+L: linux-...@vger.kernel.org
+S: Maintained
+F: Documentation/i2c/busses/i2c-nvidia-gpu
+F: drivers/i2c/busses/i2c-nvidia-gpu.c
+
 I2C MUXES
 M: Peter Rosin 
 L: linux-...@vger.kernel.org
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 451d4ae..eed827b 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -224,6 +224,15 @@ config I2C_NFORCE2_S4985
  This driver can also be built as a module.  If so, the module
  will be called i2c-nforce2-s4985.
 
+config I2C_NVIDIA_GPU
+   tristate "NVIDIA GPU I2C controller"
+   depends on PCI
+   help
+ If you say yes to this option, support will be included for the
+ NVIDIA GPU I2C controller which is used to communicate with the GPU's
+ Type-C controller. This driver can also be built as a module called
+ i2c-nvidia-gpu.
+
 config I2C_SIS5595
tristate "SiS 5595"
depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 18b26af..d499813 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -140,5 +140,6 @@ obj-$(CONFIG_I2C_SIBYTE)+= i2c-sibyte.o
 obj-$(CONFIG_I2C_XGENE_SLIMPRO) += i2c-xgene-slimpro.o
 obj-$(CONFIG_SCx200_ACB)   += scx200_acb.o
 obj-$(CONFIG_I2C_FSI)  += i2c-fsi.o
+obj-$(CONFIG_I2C_NVIDIA_GPU)   += i2c-nvidia-gpu.o
 
 ccflags-$(CONFIG_I2C_DEBUG_BUS) := -DDEBUG
diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c 
b/drivers/i2c/busses/i2c-nvidia-gpu.c
new file mode 100644
index 000..fa01187
--- /dev/null
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Nvidia GPU I2C controller Driver
+ *
+ * Copyright (C) 2018 NVIDIA Corporation. All rights reserved.
+ * Author: Ajay Gupta 
+ *
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* I2C definitions */
+#define I2C_MST_CNTL   0x0
+#define I2C_MST_CNTL_GEN_START BIT(0)
+#define I2C_MST_CNTL_GEN_STOP  BIT(1)
+#define I2C_MST_CNTL_CMD_NONE  (0 << 2)
+#define I2C_MST_CNTL_CMD_READ  (1 << 2)
+#define I2C_MST_CNTL_CMD_WRITE (2 << 2)
+#define I2C_MST_CNTL_GEN_RAB   BIT(4)
+#define I2C_MST_CNTL_BURST_SIZE_SHIFT  (6)
+#define I2C_MST_CNTL_GEN_NACK  BIT(28)
+#define I2C_MST_CNTL_STATUS(3 << 29)
+#define I2C_MST_CNTL_STATUS_OKAY   (0 << 29)
+#define I2C_MST_CNTL_STATUS_NO_ACK (1 << 29)
+#define I2C_MST_CNTL_STATUS_TIMEOUT(2 << 29)
+#define I2C_MST_CNTL_STATUS_BUS_BUSY   (3 << 29)
+#define I2C_MST_CNTL_CYCLE_TRIGGER BIT(31)
+
+#define I2C_MST_ADDR   0x04
+#define I2C_MST_ADDR_DAB   0
+
+#define I2C_MST_I2C0_TIMING0x08
+#define I2C_MST_I2C0_TIMING_SCL_PERIOD_100KHZ  0x10e
+#define I2C_MST_I2C0_TIMING_TIMEOUT_CLK_CNT16
+#define I2C_MST_I2C0_TIMING_TIMEOU

Re: usb: gadget: f_mass_storage: detect eject

2018-08-29 Thread Nuno Gonçalves
On Tue, Aug 28, 2018 at 5:09 PM Alan Stern  wrote:
> There is another way: modprobe g_file_storage with appropriate
> parameters.

Yes, but I am using a composite device, with ACM and UMS, and also
g_multi (Ethernet+ACM+UMS) is not a option since my HW does not
support enough endpoints for this configuration.

> Somewhere under /sys/devices will be a directory for the system's USB
> device controller.  Beneath that will be the gadget directory for
> f_mass_storage, and below that will be directories for the individual
> Logical Units.  Each LUN will have a sysfs entry named "file", which
> contains the name of the file or device being used as the backing
> store.

On Wed, Aug 29, 2018 at 8:58 AM Felipe Balbi
 wrote:
> you can start from /sys/class/udc/

$ find /sys | grep usb | grep file
/sys/kernel/config/usb_gadget/g1/functions/mass_storage.usb0/lun.0/file

Looks like not...

I understand this is not supported for this specific ConfigFS. But is
there other ConfigFS files that support this? I am into just patching
this downstream...

Thanks,
Nuno