Re: gadget serial driver - configuration value
Hi All,
Can you please help me understand the assignment of
bConfigurationValue in gadget serial driver.
As I had mentioned in my previous mail,
I agree that no two configurations can have same bConfigurationValue,
but in this case, the implementation is (drivers/usb/gadget/serial.c)
247 if (use_acm) {
248 serial_config_driver.label = "CDC ACM config";
249 serial_config_driver.bConfigurationValue = 2;
250 ...
253 } else if (use_obex) {
254 serial_config_driver.label = "CDC OBEX config";
255 serial_config_driver.bConfigurationValue = 3;
256 ...
259 } else {
260 serial_config_driver.label = "Generic Serial config";
261 serial_config_driver.bConfigurationValue = 1;
262...
265 }
In this case we cannot have the three configurations together.
Hence I wanted to confirm if there was any other reason as to why
different numbers were assigned.
Can you please help me on the same.
Thanks and Regards
Anjana
On Fri, Sep 5, 2014 at 6:30 PM, Anjana V Kumar wrote:
> On Thu, Sep 4, 2014 at 7:38 PM, Alan Stern wrote:
>> On Thu, 4 Sep 2014, Anjana V Kumar wrote:
>>
>>> >> We see that, the three configurations listed in serial driver (CDC
>>> >> ACM, CDC OBEX, generic serial) cannot be present together as per the
>>> >> current implementation. Is there a specific reason why the
>>> >> configuration values were set as 1, 2 and 3 instead of setting all to
>>> >> 1?
>>> >
>>> > well, setting configuration 0 means that you're not selecting any
>>> > configuration. Basically you go back to "Addressed" state, so you can't
>>> > use configuration 0 for anything, really.
>>> >
>>>
>>> Sorry for not being clear, I am not setting the configuration to 0.
>>> The question was, can we set all the three configuration values of CDC
>>> ACM, CDC OBEX, and generic serial to 1?
>>> Was there any specific reason as to why the configuration values were
>>> set as 1,2 and 3. We cannot have all three at the same time according
>>> to the current "if, elseif, else" implementation,
>>
>> No two configurations can have the same bConfigurationValue. If the
>> gadget has three different configs then it must have three different
>> config values.
>>
> I agree that no two configurations can have same bConfigurationValue,
> but in this case, the implementation is (drivers/usb/gadget/serial.c)
>
> 247 if (use_acm) {
> 248 serial_config_driver.label = "CDC ACM config";
> 249 serial_config_driver.bConfigurationValue = 2;
> 250 ...
> 253 } else if (use_obex) {
> 254 serial_config_driver.label = "CDC OBEX config";
> 255 serial_config_driver.bConfigurationValue = 3;
> 256 ...
> 259 } else {
> 260 serial_config_driver.label = "Generic Serial config";
> 261 serial_config_driver.bConfigurationValue = 1;
> 262...
> 265 }
>
> In this case we cannot have the three configurations together.
> Hence I wanted to confirm if there was any other reason as to why
> different numbers were assigned.
>
>> Alan Stern
>>
>
>
>
> --
> Anjana
--
Anjana
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] storage: Replace magic number with define in usb_stor_euscsi_init()
Hi, usb_stor_euscsi_init() calls usb_stor_control_msg() with timeout argument 5000. USB_CTRL_SET_TIMEOUT is defined to be 5000 in usb.h, so would it make sense to use that instead? Patch below if it would. Signed-off-by: Mark Knibbs --- diff -up linux-3.17-rc5/drivers/usb/storage/initializers.c.orig linux-3.17-rc5/drivers/usb/storage/initializers.c --- linux-3.17-rc5/drivers/usb/storage/initializers.c.orig 2014-09-15 01:50:12.0 +0100 +++ linux-3.17-rc5/drivers/usb/storage/initializers.c 2014-09-21 19:47:32.0 +0100 @@ -52,7 +52,7 @@ int usb_stor_euscsi_init(struct us_data us->iobuf[0] = 0x1; result = usb_stor_control_msg(us, us->send_ctrl_pipe, 0x0C, USB_RECIP_INTERFACE | USB_TYPE_VENDOR, - 0x01, 0x0, us->iobuf, 0x1, 5000); + 0x01, 0x0, us->iobuf, 0x1, USB_CTRL_SET_TIMEOUT); usb_stor_dbg(us, "-- result is %d\n", result); return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: gadget serial driver - configuration value
Anjana V Kumar wrote: > I am using 3.10 kernel. The issue is that the set_configuration(2) > is stalled, while the set_configuration(1) passes with the USB > hardware I am using. What USB hardware are you using? Why not look into the actual problem with SET_CONFIGURATION? You may discover things that would only become apparent once you have shipped devices, without a chance to solve the problem. > The question was, can we set all the three configuration values of > CDC ACM, CDC OBEX, and generic serial to 1? Try it out. Windows likes to cache descriptors, and will most certainly not like if a descriptor suddenly changes unexpectedly, so that may be the reason for distinct bConfigurationValue numbers, even though they do not strictly have to be distinct as the code is written. It does makes sense to use different values. You should investigate the problem with your USB hardware. //Peter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: FIX ME in oxu210p-hcd.c
> Subject: Re: FIX ME in oxu210p-hcd.c > > > I found a unfixed FIX ME in the file stated in my above message. I am > wondering what to set hcd->self.comtroller->dma_mask to as it's now been > defined to NULL and clearly even as a newbie this seem incorrect. > Regards Nick Usually, it is set at its controller driver or pass through through device tree or platform data. Peter
[PATCH 1/4] chipidea: usbmisc_imx: Add USB support for VF610 SoCs
From: Stefan Agner
This adds Vybrid VF610 SoC support. The IP is very similar to i.MX6,
however, the non-core registers are spread in two different register
areas. Hence we support multiple instances of the USB misc driver
and add the driver instance to the imx_usbmisc_data structure.
Signed-off-by: Peter Chen
Signed-off-by: Stefan Agner
---
.../devicetree/bindings/usb/usbmisc-imx.txt|1 +
drivers/usb/chipidea/ci_hdrc_imx.c |8 +++
drivers/usb/chipidea/ci_hdrc_imx.h |1 +
drivers/usb/chipidea/usbmisc_imx.c | 52 +++-
4 files changed, 51 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
index 97ce94e..c101a4b 100644
--- a/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
+++ b/Documentation/devicetree/bindings/usb/usbmisc-imx.txt
@@ -4,6 +4,7 @@ Required properties:
- #index-cells: Cells used to descibe usb controller index. Should be <1>
- compatible: Should be one of below:
"fsl,imx6q-usbmisc" for imx6q
+ "fsl,vf610-usbmisc" for Vybrid vf610
- reg: Should contain registers location and length
Examples:
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c
b/drivers/usb/chipidea/ci_hdrc_imx.c
index 65444b0..a7ab0f1 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -54,6 +54,7 @@ struct ci_hdrc_imx_data {
static struct imx_usbmisc_data *usbmisc_get_init_data(struct device *dev)
{
+ struct platform_device *misc_pdev;
struct device_node *np = dev->of_node;
struct of_phandle_args args;
struct imx_usbmisc_data *data;
@@ -79,8 +80,15 @@ static struct imx_usbmisc_data *usbmisc_get_init_data(struct
device *dev)
}
data->index = args.args[0];
+
+ misc_pdev = of_find_device_by_node(args.np);
of_node_put(args.np);
+ if (!misc_pdev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ data->dev = &misc_pdev->dev;
+
if (of_find_property(np, "disable-over-current", NULL))
data->disable_oc = 1;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h
b/drivers/usb/chipidea/ci_hdrc_imx.h
index 996ec93..4ed828f 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -13,6 +13,7 @@
#define __DRIVER_USB_CHIPIDEA_CI_HDRC_IMX_H
struct imx_usbmisc_data {
+ struct device *dev;
int index;
unsigned int disable_oc:1; /* over current detect disabled */
diff --git a/drivers/usb/chipidea/usbmisc_imx.c
b/drivers/usb/chipidea/usbmisc_imx.c
index 85293b8..926c997 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -57,6 +57,8 @@
#define MX6_BM_OVER_CUR_DISBIT(7)
+#define VF610_OVER_CUR_DIS BIT(7)
+
struct usbmisc_ops {
/* It's called once when probe a usb device */
int (*init)(struct imx_usbmisc_data *data);
@@ -71,10 +73,9 @@ struct imx_usbmisc {
const struct usbmisc_ops *ops;
};
-static struct imx_usbmisc *usbmisc;
-
static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
unsigned long flags;
u32 val = 0;
@@ -108,6 +109,7 @@ static int usbmisc_imx25_init(struct imx_usbmisc_data *data)
static int usbmisc_imx25_post(struct imx_usbmisc_data *data)
{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
void __iomem *reg;
unsigned long flags;
u32 val;
@@ -130,6 +132,7 @@ static int usbmisc_imx25_post(struct imx_usbmisc_data *data)
static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
unsigned long flags;
u32 val;
@@ -160,6 +163,7 @@ static int usbmisc_imx27_init(struct imx_usbmisc_data *data)
static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
void __iomem *reg = NULL;
unsigned long flags;
u32 val = 0;
@@ -204,6 +208,7 @@ static int usbmisc_imx53_init(struct imx_usbmisc_data *data)
static int usbmisc_imx6q_init(struct imx_usbmisc_data *data)
{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
unsigned long flags;
u32 reg;
@@ -221,6 +226,26 @@ static int usbmisc_imx6q_init(struct imx_usbmisc_data
*data)
return 0;
}
+static int usbmisc_vf610_init(struct imx_usbmisc_data *data)
+{
+ struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+ u32 reg;
+
+ /*
+* Vybrid only has one misc register set, but in two different
+* areas. These is reflected in two instances of this driver.
+*/
+ if (data->index >= 1)
+ return -EINVAL;
+
+ if (data->disable_oc) {
+ reg = readl(usbmisc->ba
[PATCH 0/4] chipdea patches for 3.18
Hi Greg, Below are chipdea patches for v3.18-rc1, they are based on your latest usb-next, and have verfied at i.mx6sx sabresd board. Thanks. Peter Chen (3): usb: chipidea: otg initialization is only needed when the gadget is supported usb: chipidea: enhance kernel-doc format of: add vendor prefix for Chipidea Stefan Agner (1): chipidea: usbmisc_imx: Add USB support for VF610 SoCs .../devicetree/bindings/usb/usbmisc-imx.txt|1 + .../devicetree/bindings/vendor-prefixes.txt|1 + drivers/usb/chipidea/ci.h | 14 -- drivers/usb/chipidea/ci_hdrc_imx.c |8 +++ drivers/usb/chipidea/ci_hdrc_imx.h |1 + drivers/usb/chipidea/core.c| 10 +++- drivers/usb/chipidea/usbmisc_imx.c | 52 +++- 7 files changed, 70 insertions(+), 17 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] usb: chipidea: enhance kernel-doc format
Some kernel-doc style comment are not satisfied for format, fix them.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/ci.h | 14 ++
drivers/usb/chipidea/core.c |8 +++-
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index 9563cb5..ea40626 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -99,10 +99,10 @@ enum ci_role {
/**
* struct ci_role_driver - host/gadget role driver
- * start: start this role
- * stop: stop this role
- * irq: irq handler for this role
- * name: role name string (host/gadget)
+ * @start: start this role
+ * @stop: stop this role
+ * @irq: irq handler for this role
+ * @name: role name string (host/gadget)
*/
struct ci_role_driver {
int (*start)(struct ci_hdrc *);
@@ -245,6 +245,7 @@ static inline void ci_role_stop(struct ci_hdrc *ci)
/**
* hw_read: reads from a hw register
+ * @ci: the controller
* @reg: register index
* @mask: bitfield mask
*
@@ -277,6 +278,7 @@ static inline void __hw_write(struct ci_hdrc *ci, u32 val,
/**
* hw_write: writes to a hw register
+ * @ci: the controller
* @reg: register index
* @mask: bitfield mask
* @data: new value
@@ -293,6 +295,7 @@ static inline void hw_write(struct ci_hdrc *ci, enum
ci_hw_regs reg,
/**
* hw_test_and_clear: tests & clears a hw register
+ * @ci: the controller
* @reg: register index
* @mask: bitfield mask
*
@@ -309,6 +312,7 @@ static inline u32 hw_test_and_clear(struct ci_hdrc *ci,
enum ci_hw_regs reg,
/**
* hw_test_and_write: tests & writes a hw register
+ * @ci: the controller
* @reg: register index
* @mask: bitfield mask
* @data: new value
@@ -327,6 +331,8 @@ static inline u32 hw_test_and_write(struct ci_hdrc *ci,
enum ci_hw_regs reg,
/**
* ci_otg_is_fsm_mode: runtime check if otg controller
* is in otg fsm mode.
+ *
+ * @ci: chipidea device
*/
static inline bool ci_otg_is_fsm_mode(struct ci_hdrc *ci)
{
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 579b353..ac9adcf 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -139,6 +139,8 @@ static int hw_alloc_regmap(struct ci_hdrc *ci, bool is_lpm)
/**
* hw_read_intr_enable: returns interrupt enable register
*
+ * @ci: the controller
+ *
* This function returns register data
*/
u32 hw_read_intr_enable(struct ci_hdrc *ci)
@@ -149,6 +151,8 @@ u32 hw_read_intr_enable(struct ci_hdrc *ci)
/**
* hw_read_intr_status: returns interrupt status register
*
+ * @ci: the controller
+ *
* This function returns register data
*/
u32 hw_read_intr_status(struct ci_hdrc *ci)
@@ -176,6 +180,8 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode)
/**
* hw_port_test_get: reads port test mode value
*
+ * @ci: the controller
+ *
* This function returns port test mode value
*/
u8 hw_port_test_get(struct ci_hdrc *ci)
@@ -295,7 +301,7 @@ static void hw_phymode_configure(struct ci_hdrc *ci)
/**
* ci_usb_phy_init: initialize phy according to different phy type
* @ci: the controller
- *
+ *
* This function returns an error code if usb_phy_init has failed
*/
static int ci_usb_phy_init(struct ci_hdrc *ci)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] of: add vendor prefix for Chipidea
Adds chipidea to the list of DT vendor prefixes. Signed-off-by: Peter Chen --- .../devicetree/bindings/vendor-prefixes.txt|1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt index ac7269f..28c574f 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.txt +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt @@ -29,6 +29,7 @@ calxeda Calxeda capellaCapella Microsystems, Inc cavium Cavium, Inc. cdns Cadence Design Systems Inc. +chipidea Chipidea, Inc chrp Common Hardware Reference Platform chunghwa Chunghwa Picture Tubes Ltd. cirrus Cirrus Logic, Inc. -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] usb: chipidea: otg initialization is only needed when the gadget is supported
We have only needed to enable otg initialization when both of
below conditions are satisfied:
- The controller is otg capable
- The gadget function is enabled
If the controller is otg capable, but is host-only configuration, we do
not need to access register otgsc and do any otg operations (eg, create
otg workqueue).
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/core.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 619d13e..579b353 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -658,7 +658,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
goto deinit_phy;
}
- if (ci->is_otg) {
+ if (ci->is_otg && ci->roles[CI_ROLE_GADGET]) {
/* Disable and clear all OTG irq */
hw_write_otgsc(ci, OTGSC_INT_EN_BITS | OTGSC_INT_STATUS_BITS,
OTGSC_INT_STATUS_BITS);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 0/6] usb: host: change TPL support behaviour
> > On Thu, Sep 11, 2014 at 07:57:12AM +0800, Peter Chen wrote: > > On Fri, Sep 05, 2014 at 10:08:02AM -0400, Alan Stern wrote: > > > On Fri, 5 Sep 2014, Peter Chen wrote: > > > > > > > On Thu, Sep 04, 2014 at 11:12:42AM -0400, Alan Stern wrote: > > > > > On Thu, 4 Sep 2014, Peter Chen wrote: > > > > > > > > > > > On Wed, Sep 03, 2014 at 09:48:15PM -0400, Alan Stern wrote: > > > > > > > On Thu, 4 Sep 2014, Peter Chen wrote: > > > > > > > > > > > > > > > > > > Hi Greg & Alan, any comments for this patchset? > > > > > > > > > > > > > > > > > > > > In patch 2/6, why did you move the !is_targeted(udev) > > > > > > > > > > code from > > > > > > > > > > usb_enumerate_device_otg() to usb_enumerate_device()? > > > > > > > > > > Why not leave the code where it is? > > > > > > > > > > > > > > > > > > > > > > > > > > > > TPL support is also needed for embedded host, not only otg > > > > > > > > > host. > > > > > > > > > > > > > > But usb_enumerate_device_otg() gets called for all types of > > > > > > > host, right? At least, it gets called whenever > > > > > > > usb_enumerate_device() runs. > > > > > > > > > > > > > > Yes, it contains "#ifdef CONFIG_USB_OTG". But your patch has "if > (... > > > > > > > && IS_ENABLED(CONFIG_USB_OTG))", so the behavior is the > > > > > > > same. Why move the code if there's no change in behavior? > > > > > > > > > > > > > > > > > > > At former code, the tpl support judgement (in function > > > > > > is_targeted) will only be called if CONFIG_USB_OTG is defined, > > > > > > but now, we need tpl support for all targeted hosts. > > > > > > > > > > > > Why we need IS_ENABLED(CONFIG_USB_OTG) as last conditions at > > > > > > if conditions, the reason is the operation which the B-device > > > > > > may want switch to host even if it is not at A's TPL is only for > > > > > > OTG host. > > > > > > > > > > The only side effect in is_targeted() is the dev_err() message. > > > > > Are you saying that this dev_err() message needs to appear even > > > > > when CONFIG_USB_OTG is disabled? > > > > > > > > > > > > > Yes, both embedded host and otg host CAN support TPL, if the > > > > embedded host SHOULD support TPL, it should show an err message if > > > > the unsupported device is on the port. > > > > > > > > At OTG & EH compliance test plan, > > > > > (http://www.usb.org/developers/onthego/otgeh_compliance_plan_1_2.p > > > > df) page 124, the chapter 7.3.6 A-UUT Unsupported device Message > > > > test, it needs host prints "Unsupported Device" if the attaching > > > > device is not supported (without at Targeted Peripheral List). > > > > > > Okay, then I have no objections to this patch series. > > > > > > Alan Stern > > > > Hi Greg, > > > > Will you queue this patchset? > > Ok, will do, give me a day or so, thanks, > Hi Greg, will this patchset be in your usb-next tree for v3.18? Thanks. Peter -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] phy: exynos5-drd: Fix PCS_TXDEEMPH mask
Hi Vivek, > Hi Anton, > > > On Fri, Sep 19, 2014 at 1:05 PM, Anton Tikhomirov > wrote: > > According to user manual, pcs_tx_deemph_3p5db field in PHYPARAM1 > > register is 6bits wide, so mask value should be 0x3f instead > > of 0x1f. > > > > Signed-off-by: Anton Tikhomirov > > --- > > drivers/phy/phy-exynos5-usbdrd.c |2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy- > exynos5- > > usbdrd.c > > index 392101c..f421d16 100644 > > --- a/drivers/phy/phy-exynos5-usbdrd.c > > +++ b/drivers/phy/phy-exynos5-usbdrd.c > > @@ -99,7 +99,7 @@ > > > > #define EXYNOS5_DRD_PHYPARAM1 0x20 > > > > -#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f << 0) > > +#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x3f << 0) > > use a "_3P5DB" suffix for the macro name ? may sound similar to how UM > says. Agree. Please drop this patch. I will make new one. > anyways verified from Exynos5420 user manual. > > with that change, > Reviewed-by: Vivek Gautam > > > [snip] > > > -- > Best Regards > Vivek Gautam > Samsung R&D Institute, Bangalore > India > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] phy: exynos5-drd: Fix PHYPARAM1_PCS_TXDEEMPH definition
According to user manual, pcs_tx_deemph_3p5db field in PHYPARAM1 register is 6bits wide, so mask value should be 0x3f instead of 0x1f. Additionally, this patch renames the macro to correctly reflect the field name which we see in SoC documentation. Signed-off-by: Anton Tikhomirov --- drivers/phy/phy-exynos5-usbdrd.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5- usbdrd.c index 392101c..216bbf8 100644 --- a/drivers/phy/phy-exynos5-usbdrd.c +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -99,8 +99,8 @@ #define EXYNOS5_DRD_PHYPARAM1 0x20 -#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f << 0) -#define PHYPARAM1_PCS_TXDEEMPH (0x1c) +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK (0x3f << 0) +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB (0x1c) #define EXYNOS5_DRD_PHYTERM0x24 @@ -309,8 +309,8 @@ static void exynos5_usbdrd_pipe3_init(struct exynos5_usbdrd_phy *phy_drd) reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); /* Set Tx De-Emphasis level */ - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; - reg |= PHYPARAM1_PCS_TXDEEMPH; + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); @@ -330,8 +330,8 @@ static void exynos5_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd) reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); /* Set Tx De-Emphasis level */ - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; - reg |= PHYPARAM1_PCS_TXDEEMPH; + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); /* UTMI Power Control */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] phy: exynos5-drd: Fix PHYPARAM1_PCS_TXDEEMPH definition
On Monday, September 22, 2014 10:37 AM, Anton Tikhomirov wrote: > > According to user manual, pcs_tx_deemph_3p5db field in PHYPARAM1 > register is 6bits wide, so mask value should be 0x3f instead > of 0x1f. Additionally, this patch renames the macro to correctly > reflect the field name which we see in SoC documentation. > > Signed-off-by: Anton Tikhomirov Reviewed-by: Jingoo Han Best regards, Jingoo Han > --- > drivers/phy/phy-exynos5-usbdrd.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5- > usbdrd.c > index 392101c..216bbf8 100644 > --- a/drivers/phy/phy-exynos5-usbdrd.c > +++ b/drivers/phy/phy-exynos5-usbdrd.c > @@ -99,8 +99,8 @@ > > #define EXYNOS5_DRD_PHYPARAM10x20 > > -#define PHYPARAM1_PCS_TXDEEMPH_MASK (0x1f << 0) > -#define PHYPARAM1_PCS_TXDEEMPH (0x1c) > +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK(0x3f << 0) > +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB (0x1c) > > #define EXYNOS5_DRD_PHYTERM 0x24 > > @@ -309,8 +309,8 @@ static void exynos5_usbdrd_pipe3_init(struct > exynos5_usbdrd_phy *phy_drd) > > reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > /* Set Tx De-Emphasis level */ > - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; > - reg |= PHYPARAM1_PCS_TXDEEMPH; > + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; > + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; > writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > > reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); > @@ -330,8 +330,8 @@ static void exynos5_usbdrd_utmi_init(struct > exynos5_usbdrd_phy *phy_drd) > > reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > /* Set Tx De-Emphasis level */ > - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; > - reg |= PHYPARAM1_PCS_TXDEEMPH; > + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; > + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; > writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > > /* UTMI Power Control */ > -- > 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FIX ME in oxu210p-hcd.c
On 14-09-21 07:53 PM, Peter Chen wrote:
>
>
>> Subject: Re: FIX ME in oxu210p-hcd.c
>>
>>
>> I found a unfixed FIX ME in the file stated in my above message. I am
>> wondering what to set hcd->self.comtroller->dma_mask to as it's now been
>> defined to NULL and clearly even as a newbie this seem incorrect.
>> Regards Nick
>
> Usually, it is set at its controller driver or pass through through device
> tree or
> platform data.
>
> Peter
>
Sorry Peter,
I apologize for asking for more help here but I will paste the function below
and with my changes.
Please let me known if I am wrong and how to fix it as I new here.
Sorry for Wasting Your Time,
Nick
static int oxu_reset(struct usb_hcd *hcd)
{
struct oxu_hcd *oxu = hcd_to_oxu(hcd);
int ret;
spin_lock_init(&oxu->mem_lock);
INIT_LIST_HEAD(&oxu->urb_list);
oxu->urb_len = 0;
- /* FIMXE */
+ hcd->self.controller->dma_mask = hcd->regs;
if (oxu->is_otg) {
oxu->caps = hcd->regs + OXU_OTG_CAP_OFFSET;
oxu->regs = hcd->regs + OXU_OTG_CAP_OFFSET + \
HC_LENGTH(readl(&oxu->caps->hc_capbase));
oxu->mem = hcd->regs + OXU_SPH_MEM;
} else {
oxu->caps = hcd->regs + OXU_SPH_CAP_OFFSET;
oxu->regs = hcd->regs + OXU_SPH_CAP_OFFSET + \
HC_LENGTH(readl(&oxu->caps->hc_capbase));
oxu->mem = hcd->regs + OXU_OTG_MEM;
}
oxu->hcs_params = readl(&oxu->caps->hcs_params);
oxu->sbrn = 0x20;
ret = oxu_hcd_init(hcd);
if (ret)
return ret;
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: FIX ME in oxu210p-hcd.c
> >> Subject: Re: FIX ME in oxu210p-hcd.c
> >>
> >>
> >> I found a unfixed FIX ME in the file stated in my above message. I am
> >> wondering what to set hcd->self.comtroller->dma_mask to as it's now
> >> been defined to NULL and clearly even as a newbie this seem incorrect.
> >> Regards Nick
> >
> > Usually, it is set at its controller driver or pass through through
> > device tree or platform data.
> >
> > Peter
> >
> Sorry Peter,
> I apologize for asking for more help here but I will paste the function below
> and
> with my changes.
> Please let me known if I am wrong and how to fix it as I new here.
> Sorry for Wasting Your Time,
You are welcome
> Nick
> static int oxu_reset(struct usb_hcd *hcd) {
> struct oxu_hcd *oxu = hcd_to_oxu(hcd);
> int ret;
>
> spin_lock_init(&oxu->mem_lock);
> INIT_LIST_HEAD(&oxu->urb_list);
> oxu->urb_len = 0;
>
> - /* FIMXE */
> + hcd->self.controller->dma_mask = hcd->regs;
>
It is the dma mask, not the register.
Try below patch to see if it works for you:
diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index da5fb0e..5549851 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#include
@@ -3841,6 +3842,12 @@ static int oxu_drv_probe(struct platform_device *pdev)
goto error;
}
+ ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+ if (ret) {
+ dev_err(&pdev->dev, "set dma mask error\n");
+ goto error;
+ }
+
/* Allocate a driver data struct to hold useful info for both
* SPH & OTG devices
*/
Peter
> if (oxu->is_otg) {
> oxu->caps = hcd->regs + OXU_OTG_CAP_OFFSET;
> oxu->regs = hcd->regs + OXU_OTG_CAP_OFFSET + \
> HC_LENGTH(readl(&oxu->caps->hc_capbase));
>
> oxu->mem = hcd->regs + OXU_SPH_MEM;
> } else {
> oxu->caps = hcd->regs + OXU_SPH_CAP_OFFSET;
> oxu->regs = hcd->regs + OXU_SPH_CAP_OFFSET + \
> HC_LENGTH(readl(&oxu->caps->hc_capbase));
>
> oxu->mem = hcd->regs + OXU_OTG_MEM;
> }
>
> oxu->hcs_params = readl(&oxu->caps->hcs_params);
> oxu->sbrn = 0x20;
>
> ret = oxu_hcd_init(hcd);
> if (ret)
> return ret;
>
> return 0;
> }
Re: FIX ME in oxu210p-hcd.c
On Sun, Sep 21, 2014 at 10:03:28PM -0400, nick wrote: > > > On 14-09-21 07:53 PM, Peter Chen wrote: > > > > > >> Subject: Re: FIX ME in oxu210p-hcd.c > >> > >> > >> I found a unfixed FIX ME in the file stated in my above message. I am > >> wondering what to set hcd->self.comtroller->dma_mask to as it's now been > >> defined to NULL and clearly even as a newbie this seem incorrect. > >> Regards Nick > > > > Usually, it is set at its controller driver or pass through through device > > tree or > > platform data. > > > > Peter > > > Sorry Peter, > I apologize for asking for more help here but I will paste the function below > and with my changes. > Please let me known if I am wrong and how to fix it as I new here. > Sorry for Wasting Your Time, Then please do not. Just because your other email addresses were banned from lkml, don't keep popping up and bothering people, it's rude, and will cause this address to be banned as well. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FIX ME in oxu210p-hcd.c
On 14-09-21 10:11 PM, gre...@linuxfoundation.org wrote: > On Sun, Sep 21, 2014 at 10:03:28PM -0400, nick wrote: >> >> >> On 14-09-21 07:53 PM, Peter Chen wrote: >>> >>> Subject: Re: FIX ME in oxu210p-hcd.c I found a unfixed FIX ME in the file stated in my above message. I am wondering what to set hcd->self.comtroller->dma_mask to as it's now been defined to NULL and clearly even as a newbie this seem incorrect. Regards Nick >>> >>> Usually, it is set at its controller driver or pass through through device >>> tree or >>> platform data. >>> >>> Peter >>> >> Sorry Peter, >> I apologize for asking for more help here but I will paste the function >> below and with my changes. >> Please let me known if I am wrong and how to fix it as I new here. >> Sorry for Wasting Your Time, > > Then please do not. Just because your other email addresses were banned > from lkml, don't keep popping up and bothering people, it's rude, and > will cause this address to be banned as well. > Sorry Greg, I don't want to get banned again. I was trying to help out and learn, I was apologizing not for wasting time as much as for making sure I wasn't wasting maintainer time again. I also am coming to the conclusion that my terrible patches were a waste of time and I am trying to get back into helping out. Cheers, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FIX ME in oxu210p-hcd.c
On Sun, Sep 21, 2014 at 10:17:38PM -0400, nick wrote: > > > On 14-09-21 10:11 PM, gre...@linuxfoundation.org wrote: > > On Sun, Sep 21, 2014 at 10:03:28PM -0400, nick wrote: > >> > >> > >> On 14-09-21 07:53 PM, Peter Chen wrote: > >>> > >>> > Subject: Re: FIX ME in oxu210p-hcd.c > > > I found a unfixed FIX ME in the file stated in my above message. I am > wondering what to set hcd->self.comtroller->dma_mask to as it's now been > defined to NULL and clearly even as a newbie this seem incorrect. > Regards Nick > >>> > >>> Usually, it is set at its controller driver or pass through through > >>> device tree or > >>> platform data. > >>> > >>> Peter > >>> > >> Sorry Peter, > >> I apologize for asking for more help here but I will paste the function > >> below and with my changes. > >> Please let me known if I am wrong and how to fix it as I new here. > >> Sorry for Wasting Your Time, > > > > Then please do not. Just because your other email addresses were banned > > from lkml, don't keep popping up and bothering people, it's rude, and > > will cause this address to be banned as well. > > > Sorry Greg, > I don't want to get banned again. I was trying to help out and learn, I was > apologizing not > for wasting time as much as for making sure I wasn't wasting maintainer time > again. I also > am coming to the conclusion that my terrible patches were a waste of time and > I am trying > to get back into helping out. You were asked to finish the Eudyptula challenge before bothering any other kernel developers with questions / patches. Until that happens, you are in my killfile, now with yet-another-email-address. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FIX ME in oxu210p-hcd.c
On 14-09-21 10:11 PM, Peter Chen wrote:
>
>
Subject: Re: FIX ME in oxu210p-hcd.c
I found a unfixed FIX ME in the file stated in my above message. I am
wondering what to set hcd->self.comtroller->dma_mask to as it's now
been defined to NULL and clearly even as a newbie this seem incorrect.
Regards Nick
>>>
>>> Usually, it is set at its controller driver or pass through through
>>> device tree or platform data.
>>>
>>> Peter
>>>
>> Sorry Peter,
>> I apologize for asking for more help here but I will paste the function
>> below and
>> with my changes.
>> Please let me known if I am wrong and how to fix it as I new here.
>> Sorry for Wasting Your Time,
>
> You are welcome
>
>> Nick
>> static int oxu_reset(struct usb_hcd *hcd) {
>> struct oxu_hcd *oxu = hcd_to_oxu(hcd);
>> int ret;
>>
>> spin_lock_init(&oxu->mem_lock);
>> INIT_LIST_HEAD(&oxu->urb_list);
>> oxu->urb_len = 0;
>>
>> -/* FIMXE */
>> +hcd->self.controller->dma_mask = hcd->regs;
>>
>
> It is the dma mask, not the register.
>
> Try below patch to see if it works for you:
>
> diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
> index da5fb0e..5549851 100644
> --- a/drivers/usb/host/oxu210hp-hcd.c
> +++ b/drivers/usb/host/oxu210hp-hcd.c
> @@ -37,6 +37,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -3841,6 +3842,12 @@ static int oxu_drv_probe(struct platform_device *pdev)
> goto error;
> }
>
> + ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> + if (ret) {
> + dev_err(&pdev->dev, "set dma mask error\n");
> + goto error;
> + }
> +
> /* Allocate a driver data struct to hold useful info for both
> * SPH & OTG devices
> */
>
> Peter
>
>
>> if (oxu->is_otg) {
>> oxu->caps = hcd->regs + OXU_OTG_CAP_OFFSET;
>> oxu->regs = hcd->regs + OXU_OTG_CAP_OFFSET + \
>> HC_LENGTH(readl(&oxu->caps->hc_capbase));
>>
>> oxu->mem = hcd->regs + OXU_SPH_MEM;
>> } else {
>> oxu->caps = hcd->regs + OXU_SPH_CAP_OFFSET;
>> oxu->regs = hcd->regs + OXU_SPH_CAP_OFFSET + \
>> HC_LENGTH(readl(&oxu->caps->hc_capbase));
>>
>> oxu->mem = hcd->regs + OXU_OTG_MEM;
>> }
>>
>> oxu->hcs_params = readl(&oxu->caps->hcs_params);
>> oxu->sbrn = 0x20;
>>
>> ret = oxu_hcd_init(hcd);
>> if (ret)
>> return ret;
>>
>> return 0;
>> }
Unfortunately I can't get it to apply with git apply and get the following
message,
Checking patch drivers/usb/host/oxu210hp-hcd.c...
error: while searching for:
goto error;
}
/* Allocate a driver data struct to hold useful info for both
* SPH & OTG devices
*/
error: patch failed: drivers/usb/host/oxu210hp-hcd.c:3841
error: drivers/usb/host/oxu210hp-hcd.c: patch does not apply
I will paste the version of my patch file for your troubles.
Regards Nick
diff --git a/drivers/usb/host/oxu210hp-hcd.c b/drivers/usb/host/oxu210hp-hcd.c
index da5fb0e..5549851 100644
--- a/drivers/usb/host/oxu210hp-hcd.c
+++ b/drivers/usb/host/oxu210hp-hcd.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include
#include
@@ -3841,6 +3842,12 @@ static int oxu_drv_probe(struct platform_device *pdev)
goto error;
}
+ ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+ if (ret) {
+ dev_err(&pdev->dev, "set dma mask error\n");
+ goto error;
+ }
+
/* Allocate a driver data struct to hold useful info for both
* SPH & OTG devices
*/
Re: FIX ME in oxu210p-hcd.c
On 14-09-21 10:24 PM, gre...@linuxfoundation.org wrote: > On Sun, Sep 21, 2014 at 10:17:38PM -0400, nick wrote: >> >> >> On 14-09-21 10:11 PM, gre...@linuxfoundation.org wrote: >>> On Sun, Sep 21, 2014 at 10:03:28PM -0400, nick wrote: On 14-09-21 07:53 PM, Peter Chen wrote: > > >> Subject: Re: FIX ME in oxu210p-hcd.c >> >> >> I found a unfixed FIX ME in the file stated in my above message. I am >> wondering what to set hcd->self.comtroller->dma_mask to as it's now been >> defined to NULL and clearly even as a newbie this seem incorrect. >> Regards Nick > > Usually, it is set at its controller driver or pass through through > device tree or > platform data. > > Peter > Sorry Peter, I apologize for asking for more help here but I will paste the function below and with my changes. Please let me known if I am wrong and how to fix it as I new here. Sorry for Wasting Your Time, >>> >>> Then please do not. Just because your other email addresses were banned >>> from lkml, don't keep popping up and bothering people, it's rude, and >>> will cause this address to be banned as well. >>> >> Sorry Greg, >> I don't want to get banned again. I was trying to help out and learn, I was >> apologizing not >> for wasting time as much as for making sure I wasn't wasting maintainer time >> again. I also >> am coming to the conclusion that my terrible patches were a waste of time >> and I am trying >> to get back into helping out. > > You were asked to finish the Eudyptula challenge before bothering any > other kernel developers with questions / patches. Until that happens, > you are in my killfile, now with yet-another-email-address. > > greg k-h > Greg K-H, I really don't want you to get any more upset with me then you already are. The reason I gave up on the challenge was it was mostly driver based and I wanted to learn more in other areas. If you would like to discuss with me your concerns about my work on this list and how I can aid more please let me known. Sorry , Nick -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: FIX ME in oxu210p-hcd.c
Damm, didn't configure my kill-file correctly, and this snuck in, so might as well respond... On Sun, Sep 21, 2014 at 10:36:18PM -0400, nick wrote: > On 14-09-21 10:24 PM, gre...@linuxfoundation.org wrote: > >> Sorry Greg, > >> I don't want to get banned again. I was trying to help out and learn, I > >> was apologizing not > >> for wasting time as much as for making sure I wasn't wasting maintainer > >> time again. I also > >> am coming to the conclusion that my terrible patches were a waste of time > >> and I am trying > >> to get back into helping out. > > > > You were asked to finish the Eudyptula challenge before bothering any > > other kernel developers with questions / patches. Until that happens, > > you are in my killfile, now with yet-another-email-address. > > > Greg K-H, > I really don't want you to get any more upset with me then you already are. > The reason I gave up on the > challenge was it was mostly driver based and I wanted to learn more in other > areas. If you would like to > discuss with me your concerns about my work on this list and how I can aid > more please let me known. You stopped so early in the challenge, you really have no idea what it is "mostly" about, so don't make rash statements like that. Again, unless you finish it, you will be ignored by me, and probably by everyone else as well. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] phy: exynos5-drd: Fix PHYPARAM1_PCS_TXDEEMPH definition
On Mon, Sep 22, 2014 at 7:06 AM, Anton Tikhomirov wrote: > According to user manual, pcs_tx_deemph_3p5db field in PHYPARAM1 > register is 6bits wide, so mask value should be 0x3f instead > of 0x1f. Additionally, this patch renames the macro to correctly > reflect the field name which we see in SoC documentation. > > Signed-off-by: Anton Tikhomirov > --- > drivers/phy/phy-exynos5-usbdrd.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > you can add my reviewed-by Reviewed-by: Vivek Gautam [snip] > diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5- > usbdrd.c > index 392101c..216bbf8 100644 > --- a/drivers/phy/phy-exynos5-usbdrd.c > +++ b/drivers/phy/phy-exynos5-usbdrd.c > @@ -99,8 +99,8 @@ > > #define EXYNOS5_DRD_PHYPARAM1 0x20 > > -#define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f << 0) > -#define PHYPARAM1_PCS_TXDEEMPH (0x1c) > +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK (0x3f << 0) > +#define PHYPARAM1_PCS_TXDEEMPH_3P5DB (0x1c) > > #define EXYNOS5_DRD_PHYTERM0x24 > > @@ -309,8 +309,8 @@ static void exynos5_usbdrd_pipe3_init(struct > exynos5_usbdrd_phy *phy_drd) > > reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > /* Set Tx De-Emphasis level */ > - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; > - reg |= PHYPARAM1_PCS_TXDEEMPH; > + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; > + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; > writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > > reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); > @@ -330,8 +330,8 @@ static void exynos5_usbdrd_utmi_init(struct > exynos5_usbdrd_phy *phy_drd) > > reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > /* Set Tx De-Emphasis level */ > - reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; > - reg |= PHYPARAM1_PCS_TXDEEMPH; > + reg &= ~PHYPARAM1_PCS_TXDEEMPH_3P5DB_MASK; > + reg |= PHYPARAM1_PCS_TXDEEMPH_3P5DB; > writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); > > /* UTMI Power Control */ > -- > 1.7.9.5 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Vivek Gautam Samsung R&D Institute, Bangalore India -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support
On Thu, Sep 18, 2014 at 8:20 PM, Alan Stern wrote:
> On Thu, 18 Sep 2014, Vivek Gautam wrote:
>
>> Now that we have completely moved from older USB-PHY drivers
>> to newer GENERIC-PHY drivers for PHYs available with USB controllers
>> on Exynos series of SoCs, we can remove the support for the same
>> in our host drivers too.
>>
>> We also defer the probe for our host in case we end up getting
>> EPROBE_DEFER error when getting PHYs.
>
> Better now. But I didn't notice this the first time:
>
>> + if (IS_ERR(phy)) {
>> + ret = PTR_ERR(phy);
>> + if (ret == -EPROBE_DEFER) {
>> + return ret;
>> + } else if (ret != -ENOSYS && ret != -ENODEV) {
>> + dev_err(dev,
>> + "Error retrieving usb2 phy: %d\n",
>> ret);
>> + return PTR_ERR(phy);
>
> It doesn't make any real difference, but wouldn't you prefer to say
> "return ret" here?
sure, will update this.
>
> With or without that change, for both these two patches:
>
> Acked-by: Alan Stern
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support
Now that we have completely moved from older USB-PHY drivers
to newer GENERIC-PHY drivers for PHYs available with USB controllers
on Exynos series of SoCs, we can remove the support for the same
in our host drivers too.
We also defer the probe for our host in case we end up getting
EPROBE_DEFER error when getting PHYs.
Signed-off-by: Vivek Gautam
Acked-by: Jingoo Han
Acked-by: Alan Stern
---
Changes since v4:
- returning 'ret' instead of PTR_ERR(phy), since ret is nothing but that only.
Changes since v3:
- Addressed review comments by Alan:
-- Skipped renaming 'phy_number' variable,
-- resorted to just adding minimal change required for phy assignment.
-- updated patch description to mention EPROBE_DEFER support.
drivers/usb/host/ehci-exynos.c | 74 +++-
1 file changed, 20 insertions(+), 54 deletions(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index 2eed9a4..7189f2e 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -21,11 +21,8 @@
#include
#include
#include
-#include
-#include
#include
#include
-#include
#include "ehci.h"
@@ -47,9 +44,7 @@ static struct hc_driver __read_mostly exynos_ehci_hc_driver;
struct exynos_ehci_hcd {
struct clk *clk;
- struct usb_phy *phy;
- struct usb_otg *otg;
- struct phy *phy_g[PHY_NUMBER];
+ struct phy *phy[PHY_NUMBER];
};
#define to_exynos_ehci(hcd) (struct exynos_ehci_hcd *)(hcd_to_ehci(hcd)->priv)
@@ -60,8 +55,9 @@ static int exynos_ehci_get_phy(struct device *dev,
struct device_node *child;
struct phy *phy;
int phy_number;
- int ret = 0;
+ int ret;
+ /* Get PHYs for the controller */
for_each_available_child_of_node(dev->of_node, child) {
ret = of_property_read_u32(child, "reg", &phy_number);
if (ret) {
@@ -77,31 +73,21 @@ static int exynos_ehci_get_phy(struct device *dev,
}
phy = devm_of_phy_get(dev, child, NULL);
+ exynos_ehci->phy[phy_number] = phy;
of_node_put(child);
- if (IS_ERR(phy))
- /* Lets fallback to older USB-PHYs */
- goto usb_phy_old;
- exynos_ehci->phy_g[phy_number] = phy;
- /* Make the older PHYs unavailable */
- exynos_ehci->phy = ERR_PTR(-ENXIO);
- }
-
- return 0;
-
-usb_phy_old:
- exynos_ehci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(exynos_ehci->phy)) {
- ret = PTR_ERR(exynos_ehci->phy);
- if (ret != -ENXIO && ret != -ENODEV) {
- dev_err(dev, "no usb2 phy configured\n");
- return ret;
+ if (IS_ERR(phy)) {
+ ret = PTR_ERR(phy);
+ if (ret == -EPROBE_DEFER) {
+ return ret;
+ } else if (ret != -ENOSYS && ret != -ENODEV) {
+ dev_err(dev,
+ "Error retrieving usb2 phy: %d\n", ret);
+ return ret;
+ }
}
- dev_dbg(dev, "Failed to get usb2 phy\n");
- } else {
- exynos_ehci->otg = exynos_ehci->phy->otg;
}
- return ret;
+ return 0;
}
static int exynos_ehci_phy_enable(struct device *dev)
@@ -111,16 +97,13 @@ static int exynos_ehci_phy_enable(struct device *dev)
int i;
int ret = 0;
- if (!IS_ERR(exynos_ehci->phy))
- return usb_phy_init(exynos_ehci->phy);
-
for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
- if (!IS_ERR(exynos_ehci->phy_g[i]))
- ret = phy_power_on(exynos_ehci->phy_g[i]);
+ if (!IS_ERR(exynos_ehci->phy[i]))
+ ret = phy_power_on(exynos_ehci->phy[i]);
if (ret)
for (i--; i >= 0; i--)
- if (!IS_ERR(exynos_ehci->phy_g[i]))
- phy_power_off(exynos_ehci->phy_g[i]);
+ if (!IS_ERR(exynos_ehci->phy[i]))
+ phy_power_off(exynos_ehci->phy[i]);
return ret;
}
@@ -131,14 +114,9 @@ static void exynos_ehci_phy_disable(struct device *dev)
struct exynos_ehci_hcd *exynos_ehci = to_exynos_ehci(hcd);
int i;
- if (!IS_ERR(exynos_ehci->phy)) {
- usb_phy_shutdown(exynos_ehci->phy);
- return;
- }
-
for (i = 0; i < PHY_NUMBER; i++)
- if (!IS_ERR(exynos_ehci->phy_g[i]))
- phy_power_off(exynos_ehci->phy_g[i]);
+ if (!IS_ERR(exynos_ehci->phy[i]))
+ phy_power_off(exynos_ehci->phy[i]);
}
static void exynos_setup_vbus_gpio(struct device *dev)
@@ -231,9 +209,6 @@
[PATCH v5 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support
Now that we have completely moved from older USB-PHY drivers
to newer GENERIC-PHY drivers for PHYs available with USB controllers
on Exynos series of SoCs, we can remove the support for the same
in our host drivers too.
We also defer the probe for our host in case we end up getting
EPROBE_DEFER error when getting PHYs.
Signed-off-by: Vivek Gautam
Acked-by: Jingoo Han
Acked-by: Alan Stern
---
Changes since v4:
- returning 'ret' instead of PTR_ERR(phy), since ret is nothing but that only.
Changes since v3:
- Addressed review comments by Alan:
-- Skipped renaming 'phy_number' variable,
-- resorted to just adding minimal change required for phy assignment.
-- updated patch description to mention EPROBE_DEFER support.
drivers/usb/host/ohci-exynos.c | 81 ++--
1 file changed, 20 insertions(+), 61 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 7c48e3f..d28b658 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -19,11 +19,8 @@
#include
#include
#include
-#include
-#include
#include
#include
-#include
#include "ohci.h"
@@ -38,9 +35,7 @@ static struct hc_driver __read_mostly exynos_ohci_hc_driver;
struct exynos_ohci_hcd {
struct clk *clk;
- struct usb_phy *phy;
- struct usb_otg *otg;
- struct phy *phy_g[PHY_NUMBER];
+ struct phy *phy[PHY_NUMBER];
};
static int exynos_ohci_get_phy(struct device *dev,
@@ -49,15 +44,9 @@ static int exynos_ohci_get_phy(struct device *dev,
struct device_node *child;
struct phy *phy;
int phy_number;
- int ret = 0;
+ int ret;
- /*
-* Getting generic phy:
-* We are keeping both types of phys as a part of transiting OHCI
-* to generic phy framework, so as to maintain backward compatibilty
-* with old DTB too.
-* We fallback to older USB-PHYs when we fail to get generic PHYs.
-*/
+ /* Get PHYs for the controller */
for_each_available_child_of_node(dev->of_node, child) {
ret = of_property_read_u32(child, "reg", &phy_number);
if (ret) {
@@ -73,31 +62,21 @@ static int exynos_ohci_get_phy(struct device *dev,
}
phy = devm_of_phy_get(dev, child, NULL);
+ exynos_ohci->phy[phy_number] = phy;
of_node_put(child);
- if (IS_ERR(phy))
- /* Lets fallback to older USB-PHYs */
- goto usb_phy_old;
- exynos_ohci->phy_g[phy_number] = phy;
- /* Make the older PHYs unavailable */
- exynos_ohci->phy = ERR_PTR(-ENXIO);
- }
-
- return 0;
-
-usb_phy_old:
- exynos_ohci->phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- if (IS_ERR(exynos_ohci->phy)) {
- ret = PTR_ERR(exynos_ohci->phy);
- if (ret != -ENXIO && ret != -ENODEV) {
- dev_err(dev, "no usb2 phy configured\n");
- return ret;
+ if (IS_ERR(phy)) {
+ ret = PTR_ERR(phy);
+ if (ret == -EPROBE_DEFER) {
+ return ret;
+ } else if (ret != -ENOSYS && ret != -ENODEV) {
+ dev_err(dev,
+ "Error retrieving usb2 phy: %d\n", ret);
+ return ret;
+ }
}
- dev_dbg(dev, "Failed to get usb2 phy\n");
- } else {
- exynos_ohci->otg = exynos_ohci->phy->otg;
}
- return ret;
+ return 0;
}
static int exynos_ohci_phy_enable(struct device *dev)
@@ -107,16 +86,13 @@ static int exynos_ohci_phy_enable(struct device *dev)
int i;
int ret = 0;
- if (!IS_ERR(exynos_ohci->phy))
- return usb_phy_init(exynos_ohci->phy);
-
for (i = 0; ret == 0 && i < PHY_NUMBER; i++)
- if (!IS_ERR(exynos_ohci->phy_g[i]))
- ret = phy_power_on(exynos_ohci->phy_g[i]);
+ if (!IS_ERR(exynos_ohci->phy[i]))
+ ret = phy_power_on(exynos_ohci->phy[i]);
if (ret)
for (i--; i >= 0; i--)
- if (!IS_ERR(exynos_ohci->phy_g[i]))
- phy_power_off(exynos_ohci->phy_g[i]);
+ if (!IS_ERR(exynos_ohci->phy[i]))
+ phy_power_off(exynos_ohci->phy[i]);
return ret;
}
@@ -127,14 +103,9 @@ static void exynos_ohci_phy_disable(struct device *dev)
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
int i;
- if (!IS_ERR(exynos_ohci->phy)) {
- usb_phy_shutdown(exynos_ohci->phy);
- return;
- }
-
for (i = 0; i < PHY_NUMBER; i++)
- if (!
