Re: [PATCH v2 5/8] usb: ohci-da8xx: add vbus and overcurrent gpios
On 11/02/19 4:06 PM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > There are two users upstream which register external callbacks for > switching the port power on/off and overcurrent protection. Both > users only use two GPIOs for that. Instead of having that functionality > in the board files, move the logic into the OHCI driver - including > the interrupt handler for overcurrent detection. > > Signed-off-by: Bartosz Golaszewski > Acked-by: Alan Stern > --- > drivers/usb/host/ohci-da8xx.c | 99 ++- > 1 file changed, 50 insertions(+), 49 deletions(-) > > diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c > index e8ede0b5e3f0..80c23307fbfe 100644 > --- a/drivers/usb/host/ohci-da8xx.c > +++ b/drivers/usb/host/ohci-da8xx.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > #include > #include Added the new include in alphabetical order. With that minor fix, series is applied to my v5.1/soc branch. Thanks, Sekhar
Re: [PATCH 5/8] usb: ohci-da8xx: add vbus and overcurrent gpios
Hi Bartosz,
On 05/02/19 3:55 PM, Bartosz Golaszewski wrote:
> +static irqreturn_t ohci_da8xx_oc_handler(int irq, void *data)
> +{
> + struct da8xx_ohci_hcd *da8xx_ohci = data;
> +
> + if (gpiod_get_value_cansleep(da8xx_ohci->oc_gpio))
> + gpiod_set_value_cansleep(da8xx_ohci->vbus_gpio, 0);
> +
> + return IRQ_HANDLED;
> +}
Its pretty strange to see gpiod_get_value_cansleep() being called from
irq context, although I agree right now it uses SoC GPIOs so it should
actually never sleep.
Isn't it better to use gpiod_get_value() instead so you get a warning on
incorrect usage?
Thanks,
Sekhar
Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API
On 14/12/18 4:56 PM, Felipe Balbi wrote: > Hi, > > Sekhar Nori writes: >>>>>>> All this should be part of comments in code along with information about >>>>>>> controller versions which suffer from the errata. >>>>>>> >>>>>>> Is there a version of controller available which does not have the >>>>>>> defect? Is there a future plan to fix this? >>>>>>> >>>>>>> If any of that is yes, you probably want to handle this with runtime >>>>>>> detection of version (like done with DWC3_REVISION_XXX macros). >>>>>>> Sometimes the hardware-read versions themselves are incorrect, so its >>>>>>> better to introduce a version specific compatible too like >>>>>>> "cdns,usb-1.0.0" (as hinted to by Rob Herring as well). >>>>>>> >>>>>> >>>>>> custom match_ep is used and works with all versions of the gen1 >>>>>> controller. Future (gen2) releases of the controller won’t have such >>>>>> limitation but there is no plan to change current (gen1) functionality >>>>>> of the controller. >>>>>> >>>>>> I will add comment before cdns3_gadget_match_ep function. >>>>>> Also I will change cdns,usb3 to cdns,usb3-1.0.0 and add additional >>>>>> cdns,usb3-1.0.1 compatible. >>>>>> >>>>>> cdns,usb3-1.0.1 will be for current version of controller which I use. >>>>>> cdns,usb3-1.0.0 will be for older version - Peter Chan platform. >>>>>> I now that I have some changes in controller, and one of them require >>>>>> some changes in DRD driver. It will be safer to add two separate >>>>>> version in compatibles. >>>>>> >>>>> >>>>> Pawel, could we have correct register to show controller version? It is >>>>> better we could version judgement at runtime instead of static compatible. >>>> >>>> Agree with detecting IP version at runtime. >>>> >>>> But please have some indication of version in compatible string too, >>> >>> why? Runtime detection by revision register should be the way to go if >>> the HW provides it. Why duplicate the information in compatible string? >>> >>>> especially since you already know there is going to be another revision >>>> of hardware. It has the advantage that one can easily grep to see which >>>> hardware is running current version of controller without having access >>>> to the hardware itself. Becomes useful later on when its time to >>>> clean-up unused code when boards become obsolete or for requesting >>>> testing help. >>> >>> This doesn't sound like a very strong argument, actually. Specially when >>> you consider that, since driver will do revision checking based on >>> revision register, you already have strings to grep. Moreover, we don't >>> usually drop support just like that. >> >> AFAICS, it is impossible to know just by grep'ing if there is any >> hardware still supported in kernel and using DWC3_REVISION_194A, for >> example. > > but why do you even care? When, for example, its coming in the way of some clean-up I am attempting to do. > >> If we are never going to drop support for any revision, this does not >> matter much. >> >> Also, once you have the controller supported behind PCI, then I guess >> you are pretty much tied to having to read hardware revision at runtime. > > that's another argument *for* using runtime detection, not against it. I know :). I should have stated that in last e-mail itself, I am okay with just runtime detection. Thanks, Sekhar
Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API
Hi Felipe, On 14/12/18 4:17 PM, Felipe Balbi wrote: > Hi, > > Sekhar Nori writes: > > > >>>>> All this should be part of comments in code along with information about >>>>> controller versions which suffer from the errata. >>>>> >>>>> Is there a version of controller available which does not have the >>>>> defect? Is there a future plan to fix this? >>>>> >>>>> If any of that is yes, you probably want to handle this with runtime >>>>> detection of version (like done with DWC3_REVISION_XXX macros). >>>>> Sometimes the hardware-read versions themselves are incorrect, so its >>>>> better to introduce a version specific compatible too like >>>>> "cdns,usb-1.0.0" (as hinted to by Rob Herring as well). >>>>> >>>> >>>> custom match_ep is used and works with all versions of the gen1 >>>> controller. Future (gen2) releases of the controller won’t have such >>>> limitation but there is no plan to change current (gen1) functionality >>>> of the controller. >>>> >>>> I will add comment before cdns3_gadget_match_ep function. >>>> Also I will change cdns,usb3 to cdns,usb3-1.0.0 and add additional >>>> cdns,usb3-1.0.1 compatible. >>>> >>>> cdns,usb3-1.0.1 will be for current version of controller which I use. >>>> cdns,usb3-1.0.0 will be for older version - Peter Chan platform. >>>> I now that I have some changes in controller, and one of them require >>>> some changes in DRD driver. It will be safer to add two separate >>>> version in compatibles. >>>> >>> >>> Pawel, could we have correct register to show controller version? It is >>> better we could version judgement at runtime instead of static compatible. >> >> Agree with detecting IP version at runtime. >> >> But please have some indication of version in compatible string too, > > why? Runtime detection by revision register should be the way to go if > the HW provides it. Why duplicate the information in compatible string? > >> especially since you already know there is going to be another revision >> of hardware. It has the advantage that one can easily grep to see which >> hardware is running current version of controller without having access >> to the hardware itself. Becomes useful later on when its time to >> clean-up unused code when boards become obsolete or for requesting >> testing help. > > This doesn't sound like a very strong argument, actually. Specially when > you consider that, since driver will do revision checking based on > revision register, you already have strings to grep. Moreover, we don't > usually drop support just like that. AFAICS, it is impossible to know just by grep'ing if there is any hardware still supported in kernel and using DWC3_REVISION_194A, for example. If we are never going to drop support for any revision, this does not matter much. Also, once you have the controller supported behind PCI, then I guess you are pretty much tied to having to read hardware revision at runtime. > Personally, I wouldn't bother. Just like dwc3 never bothered and nothing > bad came about because of it. Well, there are quirks which are > undetectable and for those we have quirk flags. I much prefer that > arrangement. Yes, quirk flags will work too for undetectable quirks. Thanks, Sekhar
Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API
On 14/12/18 7:04 AM, Peter Chen wrote:
> On Wed, Dec 12, 2018 at 3:49 AM Pawel Laszczak wrote:
>>
>> Hi,
>>
>>> On 10/12/18 7:42 AM, Peter Chen wrote:
>> +static struct usb_ep *cdns3_gadget_match_ep(struct usb_gadget *gadget,
>> + struct usb_endpoint_descriptor
>> *desc,
>> + struct
>> usb_ss_ep_comp_descriptor *comp_desc)
>> +{
>> + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget);
>> + struct cdns3_endpoint *priv_ep;
>> + unsigned long flags;
>> +
>> + priv_ep = cdns3_find_available_ss_ep(priv_dev, desc);
>> + if (IS_ERR(priv_ep)) {
>> + dev_err(&priv_dev->dev, "no available ep\n");
>> + return NULL;
>> + }
>> +
>> + dev_dbg(&priv_dev->dev, "match endpoint: %s\n", priv_ep->name);
>> +
>> + spin_lock_irqsave(&priv_dev->lock, flags);
>> + priv_ep->endpoint.desc = desc;
>> + priv_ep->dir = usb_endpoint_dir_in(desc) ? USB_DIR_IN :
>> USB_DIR_OUT;
>> + priv_ep->type = usb_endpoint_type(desc);
>> +
>> + list_add_tail(&priv_ep->ep_match_pending_list,
>> + &priv_dev->ep_match_list);
>> + spin_unlock_irqrestore(&priv_dev->lock, flags);
>> + return &priv_ep->endpoint;
>> +}
> Why do you need a custom match_ep?
> doesn't usb_ep_autoconfig suffice?
>
> You can check if EP is claimed or not by checking the ep->claimed flag.
>
It is a special requirement for this IP, the EP type and MaxPacketSize
changing can't be done at runtime, eg, at ep->enable. See below commit
for detail.
usb: cdns3: gadget: configure all endpoints before set configuration
Cadence IP has one limitation that all endpoints must be configured
(Type & MaxPacketSize) before setting configuration through hardware
register, it means we can't change endpoints configuration after
set_configuration.
In this patch, we add non-control endpoint through
usb_ss->ep_match_list,
which is added when the gadget driver uses usb_ep_autoconfig to
configure
specific endpoint; When the udc driver receives set_configurion
request,
it goes through usb_ss->ep_match_list, and configure all endpoints
accordingly.
At usb_ep_ops.enable/disable, we only enable and disable endpoint
through
ep_cfg register which can be changed after set_configuration, and do
some software operation accordingly.
>>>
>>> All this should be part of comments in code along with information about
>>> controller versions which suffer from the errata.
>>>
>>> Is there a version of controller available which does not have the
>>> defect? Is there a future plan to fix this?
>>>
>>> If any of that is yes, you probably want to handle this with runtime
>>> detection of version (like done with DWC3_REVISION_XXX macros).
>>> Sometimes the hardware-read versions themselves are incorrect, so its
>>> better to introduce a version specific compatible too like
>>> "cdns,usb-1.0.0" (as hinted to by Rob Herring as well).
>>>
>>
>> custom match_ep is used and works with all versions of the gen1
>> controller. Future (gen2) releases of the controller won’t have such
>> limitation but there is no plan to change current (gen1) functionality
>> of the controller.
>>
>> I will add comment before cdns3_gadget_match_ep function.
>> Also I will change cdns,usb3 to cdns,usb3-1.0.0 and add additional
>> cdns,usb3-1.0.1 compatible.
>>
>> cdns,usb3-1.0.1 will be for current version of controller which I use.
>> cdns,usb3-1.0.0 will be for older version - Peter Chan platform.
>> I now that I have some changes in controller, and one of them require
>> some changes in DRD driver. It will be safer to add two separate
>> version in compatibles.
>>
>
> Pawel, could we have correct register to show controller version? It is
> better we could version judgement at runtime instead of static compatible.
Agree with detecting IP version at runtime.
But please have some indication of version in compatible string too,
especially since you already know there is going to be another revision
of hardware. It has the advantage that one can easily grep to see which
hardware is running current version of controller without having access
to the hardware itself. Becomes useful later on when its time to
clean-up unused code when boards become obsolete or for requesting
testing help.
Thanks,
Sekhar
Re: [RFC PATCH v2 08/15] usb:cdns3: Implements device operations part of the API
On 10/12/18 7:42 AM, Peter Chen wrote:
>>> +static struct usb_ep *cdns3_gadget_match_ep(struct usb_gadget *gadget,
>>> + struct usb_endpoint_descriptor
>>> *desc,
>>> + struct usb_ss_ep_comp_descriptor
>>> *comp_desc)
>>> +{
>>> + struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget);
>>> + struct cdns3_endpoint *priv_ep;
>>> + unsigned long flags;
>>> +
>>> + priv_ep = cdns3_find_available_ss_ep(priv_dev, desc);
>>> + if (IS_ERR(priv_ep)) {
>>> + dev_err(&priv_dev->dev, "no available ep\n");
>>> + return NULL;
>>> + }
>>> +
>>> + dev_dbg(&priv_dev->dev, "match endpoint: %s\n", priv_ep->name);
>>> +
>>> + spin_lock_irqsave(&priv_dev->lock, flags);
>>> + priv_ep->endpoint.desc = desc;
>>> + priv_ep->dir = usb_endpoint_dir_in(desc) ? USB_DIR_IN : USB_DIR_OUT;
>>> + priv_ep->type = usb_endpoint_type(desc);
>>> +
>>> + list_add_tail(&priv_ep->ep_match_pending_list,
>>> + &priv_dev->ep_match_list);
>>> + spin_unlock_irqrestore(&priv_dev->lock, flags);
>>> + return &priv_ep->endpoint;
>>> +}
>> Why do you need a custom match_ep?
>> doesn't usb_ep_autoconfig suffice?
>>
>> You can check if EP is claimed or not by checking the ep->claimed flag.
>>
> It is a special requirement for this IP, the EP type and MaxPacketSize
> changing can't be done at runtime, eg, at ep->enable. See below commit
> for detail.
>
> usb: cdns3: gadget: configure all endpoints before set configuration
>
> Cadence IP has one limitation that all endpoints must be configured
> (Type & MaxPacketSize) before setting configuration through hardware
> register, it means we can't change endpoints configuration after
> set_configuration.
>
> In this patch, we add non-control endpoint through usb_ss->ep_match_list,
> which is added when the gadget driver uses usb_ep_autoconfig to configure
> specific endpoint; When the udc driver receives set_configurion request,
> it goes through usb_ss->ep_match_list, and configure all endpoints
> accordingly.
>
> At usb_ep_ops.enable/disable, we only enable and disable endpoint through
> ep_cfg register which can be changed after set_configuration, and do
> some software operation accordingly.
All this should be part of comments in code along with information about
controller versions which suffer from the errata.
Is there a version of controller available which does not have the
defect? Is there a future plan to fix this?
If any of that is yes, you probably want to handle this with runtime
detection of version (like done with DWC3_REVISION_XXX macros).
Sometimes the hardware-read versions themselves are incorrect, so its
better to introduce a version specific compatible too like
"cdns,usb-1.0.0" (as hinted to by Rob Herring as well).
Thanks,
Sekhar
Re: [PATCH 1/3] usb: musb: gadget: fix to_musb_request() to not return NULL
On Thursday 26 April 2018 11:23 AM, Sekhar Nori wrote:
> On Thursday 19 April 2018 01:35 AM, Bin Liu wrote:
>> The gadget function drivers should ensure the usb_request parameter
>> passed in is not NULL. UDC core doesn't check if it is NULL, so MUSB
>> driver shouldn't have to check it either.
>>
>> Convert to_musb_request() to a simple macro to not directly return NULL
>> to avoid warnings from code static analysis tools.
>>
>> Signed-off-by: Bin Liu
>> ---
>> drivers/usb/musb/musb_gadget.h | 5 +
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
>> index 9c34aca06db6..466877b471ef 100644
>> --- a/drivers/usb/musb/musb_gadget.h
>> +++ b/drivers/usb/musb/musb_gadget.h
>> @@ -60,10 +60,7 @@ struct musb_request {
>> enum buffer_map_state map_state;
>> };
>>
>> -static inline struct musb_request *to_musb_request(struct usb_request *req)
>> -{
>> -return req ? container_of(req, struct musb_request, request) : NULL;
>> -}
>> +#define to_musb_request(r) (container_of(r, struct musb_request, request))
>
> Unnecessary parens over container_of
And 'r' can actually be parenthesized for safety.
Thanks,
Sekhar
--
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 1/3] usb: musb: gadget: fix to_musb_request() to not return NULL
On Thursday 19 April 2018 01:35 AM, Bin Liu wrote:
> The gadget function drivers should ensure the usb_request parameter
> passed in is not NULL. UDC core doesn't check if it is NULL, so MUSB
> driver shouldn't have to check it either.
>
> Convert to_musb_request() to a simple macro to not directly return NULL
> to avoid warnings from code static analysis tools.
>
> Signed-off-by: Bin Liu
> ---
> drivers/usb/musb/musb_gadget.h | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget.h b/drivers/usb/musb/musb_gadget.h
> index 9c34aca06db6..466877b471ef 100644
> --- a/drivers/usb/musb/musb_gadget.h
> +++ b/drivers/usb/musb/musb_gadget.h
> @@ -60,10 +60,7 @@ struct musb_request {
> enum buffer_map_state map_state;
> };
>
> -static inline struct musb_request *to_musb_request(struct usb_request *req)
> -{
> - return req ? container_of(req, struct musb_request, request) : NULL;
> -}
> +#define to_musb_request(r) (container_of(r, struct musb_request, request))
Unnecessary parens over container_of
Thanks,
Sekhar
--
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 v2 0/4] Update MUSB CPPI 4.1 driver to correctly manage the DA8xx
On Thursday 21 September 2017 12:56 AM, Alexandre Bailon wrote: > A couple of weeks ago, Sekhar reported a warning issue happening in > CPPI 4.1. The teardown sequence was not correctly programmed. > The caused was a couple of difference between the DSPS and the DA8xx. > These differences are the way to program the autoreq, the teardown and > the DMA mode. > This series intends to fix the teardown and to correctly program > the DMA mode. > > This series has been roughly tested. > I have only tried it on the DA580 LCK and the BeagleBone Black. > Only the device mode (msc and ecm) have been tested. Tested-by: Sekhar Nori Tested on DA850 LCDK using g_ether (module probe and removal) and g_ether (fast ping traffic test). It will be nice if this can go into v4.14-rc cycle as these are important fixes for DA850 platform. Thanks, Sekhar -- 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 1/2] usb: musb: musb_cppi41: Fix the address of teardown and autoreq registers
On Monday 14 August 2017 07:06 PM, Sekhar Nori wrote: > On Monday 14 August 2017 07:01 PM, Sekhar Nori wrote: >> Hi, >> >> On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: >>> The DA8xx and DSPS platforms don't use the same address for few registers. >>> On Da8xx, this is causing some issues (e.g. teardown that doesn't work). >>> Configure the address of the register during the init and use them instead >>> of constants. >>> >>> Reported-by: nsek...@ti.com > > Reported-by: Sekhar Nori Tested-by: Sekhar Nori Hi Bin, Do you have any additional comments on this series or are you waiting for v2 to be posted? Thanks, Sekhar -- 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 1/2] usb: musb: musb_cppi41: Fix the address of teardown and autoreq registers
On Monday 14 August 2017 07:01 PM, Sekhar Nori wrote: > Hi, > > On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: >> The DA8xx and DSPS platforms don't use the same address for few registers. >> On Da8xx, this is causing some issues (e.g. teardown that doesn't work). >> Configure the address of the register during the init and use them instead >> of constants. >> >> Reported-by: nsek...@ti.com Reported-by: Sekhar Nori >> Signed-off-by: Alexandre Bailon >> --- >> drivers/usb/musb/musb_cppi41.c | 23 +++ >> 1 file changed, 19 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c >> index ba255280a624..dbff0e0a4ff5 100644 >> --- a/drivers/usb/musb/musb_cppi41.c >> +++ b/drivers/usb/musb/musb_cppi41.c >> @@ -26,6 +26,9 @@ >> >> #define MUSB_DMA_NUM_CHANNELS 15 >> >> +#define DA8XX_USB_AUTOREQ_REG 0x14 >> +#define DA8XX_USB_TEARDOWN_REG 0x1c > > Nice catch. I noticed that the USB_CTRL_TX_MODE and USB_CTRL_RX_MODE are > incorrect for DA8xx too. Perhaps those should be fixed too. Ah, just read 2/2 now, and seems like thats exactly what you are handling there. Thanks, Sekhar -- 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 1/2] usb: musb: musb_cppi41: Fix the address of teardown and autoreq registers
Hi, On Sunday 13 August 2017 05:34 PM, Alexandre Bailon wrote: > The DA8xx and DSPS platforms don't use the same address for few registers. > On Da8xx, this is causing some issues (e.g. teardown that doesn't work). > Configure the address of the register during the init and use them instead > of constants. > > Reported-by: nsek...@ti.com > Signed-off-by: Alexandre Bailon > --- > drivers/usb/musb/musb_cppi41.c | 23 +++ > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c > index ba255280a624..dbff0e0a4ff5 100644 > --- a/drivers/usb/musb/musb_cppi41.c > +++ b/drivers/usb/musb/musb_cppi41.c > @@ -26,6 +26,9 @@ > > #define MUSB_DMA_NUM_CHANNELS 15 > > +#define DA8XX_USB_AUTOREQ_REG0x14 > +#define DA8XX_USB_TEARDOWN_REG 0x1c Nice catch. I noticed that the USB_CTRL_TX_MODE and USB_CTRL_RX_MODE are incorrect for DA8xx too. Perhaps those should be fixed too. Also, since the original code does not use the _REG suffix for register offsets, perhaps the new defines should avoid those as well for consistency. Thanks, Sekhar -- 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] usb: gadget: f_uac2: calculate wMaxPacketSize before endpoint match
On Tuesday 08 August 2017 07:50 PM, Roger Quadros wrote: > On 17/05/17 11:15, Sekhar Nori wrote: >> Calculate wMaxPacketSize before endpoint matching the >> descriptor is found. >> >> This allows audio gadget to be used with controllers >> which have a shortage or unavailability of endpoints >> that can handle max packet size of 1023 (FS) or 1024 >> (HS). >> >> With this audio gadget can be used on TI's OMAP-L138 SoC >> which has a MUSB HS controller with endpoints having max >> packet size much less than 1023 or 1024. See mode_2_cfg in >> drivers/usb/musb/musb_core.c >> >> Signed-off-by: Sekhar Nori > > Acked-by: Roger Quadros Thanks Roger! This has already made it to upstream as commit 0db56e43359c47ff184ceaf8b04b664d997bff88. Regards, Sekhar -- 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: Warning dump on OMAP-L138 when g_zero module is removed
Hi Alexandre, On Sunday 06 August 2017 01:37 AM, Alexandre Bailon wrote: > Hi Sekhar, > On 07/21/2017 05:18 PM, Alexandre Bailon wrote: >> Hi Sekhar, >> >> On 07/10/2017 01:00 PM, Sekhar Nori wrote: >>> On Thursday 06 July 2017 10:43 PM, Alexandre Bailon wrote: >>>> On 06/29/2017 03:50 PM, Sekhar Nori wrote: >>>>> Hi Alexandre, Bin, >>>>> >>>>> With latest linux-next, I see a warning dump when I remove g_zero[1] on >>>>> OMAP-L138 LCDK board. I am building the kernel with davinci_all_defconfig. >>>>> >>>>> It is not present in latest mainline because the warnings are introduced >>>>> with CPPI DMA getting enabled in latest -next. >>>>> >>>>> Subsequent insertion of g_ether leads to a warning too[2], although the >>>>> gadget seems to work (ping test). >>>>> >>>>> Since these are pretty annoying, it will be nice to get rid of them. I >>>>> have not been able to debug any further. But if you have >>>>> ideas/experiments to try, I can do that. >>> >>>> I got a lot of these warnings during development but it was only for the >>>> host mode. >>>> If I remember correctly, the cause was a race between the teardown >>>> function and the interrupt handler. >>>> The teardwon descriptor was pop from the queue by the interrupt handler, >>>> preventing cppi41_tear_down_chan() to get it, which will result after >>>> some retries to a couple of warnings. >>>> >>>> I will take a look to see if that is the same issue. >> That is not the same issue. Still, for some reasons, we never get the >> teardown descriptor. >> Two possibilities: >> - Like the issue I had, the teardown is pop at the wrong place. >> - The teardown doesn't complete or the descriptor is not queued. > The teardown descriptor is not queued. I have figured out the reason. > The function cppi41_dma_channel_abort() enable the teardown by setting the > bit corresponding to the endpoint number. > The address of the register is USB_TDOWN but actually the address is wrong in > the case of DA8xx. > Using the right address (0x1c) fix the teardown warnings. > I will work on a fix. >> I think we should eliminate the first one before to work on the second one. >> I think adding some logs to cppi41_pop_desc() could help to figure out >> what is happening. >>> >>> Thanks! I also see similar warnings under fast ping traffic and g_ether >>> inserted on LCDK. They dont come immediately though. Only after an hour >>> or so under traffic. > The fix should also fix this issue. >> I would not have expected to have them during a ping but this probably >> happens because some USB packet have timed out, which cause may a teardown. >>> >>> I can those provide logs too if its going to be helpful. >> Currently, I think we should focus on the warnings that happens during >> the rmmod as they are always reproducible. >> >> Thanks, >> Alexandre >> > Note that the teardown is not the only thing that doesn't work correctly. > I think same kind of issue should apply to the autoreq and the dma mode. > I will work on a patch that should solve all of them. Glad you found the issue. When you have a patch, I will be happy to test it out. Thanks, Sekhar -- 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: Warning dump on OMAP-L138 when g_zero module is removed
On Thursday 06 July 2017 10:43 PM, Alexandre Bailon wrote: > On 06/29/2017 03:50 PM, Sekhar Nori wrote: >> Hi Alexandre, Bin, >> >> With latest linux-next, I see a warning dump when I remove g_zero[1] on >> OMAP-L138 LCDK board. I am building the kernel with davinci_all_defconfig. >> >> It is not present in latest mainline because the warnings are introduced >> with CPPI DMA getting enabled in latest -next. >> >> Subsequent insertion of g_ether leads to a warning too[2], although the >> gadget seems to work (ping test). >> >> Since these are pretty annoying, it will be nice to get rid of them. I >> have not been able to debug any further. But if you have >> ideas/experiments to try, I can do that. > I got a lot of these warnings during development but it was only for the > host mode. > If I remember correctly, the cause was a race between the teardown > function and the interrupt handler. > The teardwon descriptor was pop from the queue by the interrupt handler, > preventing cppi41_tear_down_chan() to get it, which will result after > some retries to a couple of warnings. > > I will take a look to see if that is the same issue. Thanks! I also see similar warnings under fast ping traffic and g_ether inserted on LCDK. They dont come immediately though. Only after an hour or so under traffic. I can those provide logs too if its going to be helpful. Thanks, Sekhar -- 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
Warning dump on OMAP-L138 when g_zero module is removed
Hi Alexandre, Bin, With latest linux-next, I see a warning dump when I remove g_zero[1] on OMAP-L138 LCDK board. I am building the kernel with davinci_all_defconfig. It is not present in latest mainline because the warnings are introduced with CPPI DMA getting enabled in latest -next. Subsequent insertion of g_ether leads to a warning too[2], although the gadget seems to work (ping test). Since these are pretty annoying, it will be nice to get rid of them. I have not been able to debug any further. But if you have ideas/experiments to try, I can do that. Thanks, Sekhar [1] root@omapl138-lcdk:~# modprobe g_zero zero gadget: Gadget Zero, version: Cinco de Mayo 2008 zero gadget: zero ready musb_g_ep0_irq 712: SetupEnd came in a wrong ep0stage setup root@omapl138-lcdk:~# zero gadget: high-speed config #3: source/sink root@omapl138-lcdk:~# modprobe -r g_zero [ cut here ] WARNING: CPU: 0 PID: 464 at drivers/dma/cppi41.c:694 cppi41_stop_chan+0x208/0x338 [cppi41] Modules linked in: usb_f_ss_lb g_zero(-) libcomposite configfs ofpart davinci_nand nand nand_ecc cppi41 mtd vpif_display tvp514x vpif_capture videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_fwnode v4l2_common vid4 CPU: 0 PID: 464 Comm: modprobe Tainted: GW 4.12.0-rc7-next-20170629-10695-g3fb8ba638d53 #76 Hardware name: Generic DA850/OMAP-L138/AM18x Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:0009 r6: r5:bf3a19f0 r4: [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (__warn+0xdc/0x104) [] (__warn) from [] (warn_slowpath_null+0x28/0x30) r9:00d8 r8:bf2d9e00 r7:0001 r6:c5b5b010 r5:c5b5b014 r4:c7b2b8a0 [] (warn_slowpath_null) from [] (cppi41_stop_chan+0x208/0x338 [cppi41]) [] (cppi41_stop_chan [cppi41]) from [] (cppi41_dma_channel_abort+0x118/0x290 [musb_hdrc]) r7:c5848010 r6:0002 r5:0001 r4:c7077454 [] (cppi41_dma_channel_abort [musb_hdrc]) from [] (musb_gadget_disable+0xfc/0x240 [musb_hdrc]) r10:c7077000 r9:fee00410 r8:2093 r7:c5848010 r6:bf2d9de4 r5:c5848010 r4:c5848410 [] (musb_gadget_disable [musb_hdrc]) from [] (usb_ep_disable+0x30/0x3c [udc_core]) r10: r9:c7144000 r8:c000a604 r7:c7334ba0 r6:c584846c r5: r4:c5848410 r3:bf2d4ad0 [] (usb_ep_disable [udc_core]) from [] (disable_endpoints+0x20/0x4c [usb_f_ss_lb]) r5: r4: [] (disable_endpoints [usb_f_ss_lb]) from [] (disable_source_sink+0x34/0x3c [usb_f_ss_lb]) r7:c7334ba0 r6: r5:6013 r4:c7159e40 [] (disable_source_sink [usb_f_ss_lb]) from [] (sourcesink_disable+0x10/0x14 [usb_f_ss_lb]) [] (sourcesink_disable [usb_f_ss_lb]) from [] (composite_disconnect+0x58/0xd0 [libcomposite]) [] (composite_disconnect [libcomposite]) from [] (usb_gadget_remove_driver+0x58/0xa8 [udc_core]) r7:0081 r6:0800 r5:c5849138 r4:c7b52600 [] (usb_gadget_remove_driver [udc_core]) from [] (usb_gadget_unregister_driver+0x68/0xd0 [udc_core]) r5:bf3e591c r4:c7b52600 [] (usb_gadget_unregister_driver [udc_core]) from [] (usb_composite_unregister+0x14/0x18 [libcomposite]) r5:0003af5c r4:bf3e59e0 [] (usb_composite_unregister [libcomposite]) from [] (zero_driver_exit+0x14/0x1c [g_zero]) [] (zero_driver_exit [g_zero]) from [] (SyS_delete_module+0x168/0x1e8) [] (SyS_delete_module) from [] (ret_fast_syscall+0x0/0x38) r6: r5:0003af5c r4:0003af20 ---[ end trace 3d9a1549a3b9c32f ]--- [ cut here ] WARNING: CPU: 0 PID: 464 at drivers/dma/cppi41.c:700 cppi41_stop_chan+0x248/0x338 [cppi41] Modules linked in: usb_f_ss_lb g_zero(-) libcomposite configfs ofpart davinci_nand nand nand_ecc cppi41 mtd vpif_display tvp514x vpif_capture videobuf2_dma_contig videobuf2_memops videobuf2_v4l2 videobuf2_core v4l2_fwnode v4l2_common vid4 CPU: 0 PID: 464 Comm: modprobe Tainted: GW 4.12.0-rc7-next-20170629-10695-g3fb8ba638d53 #76 Hardware name: Generic DA850/OMAP-L138/AM18x Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:0009 r6: r5:bf3a19f0 r4: [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (__warn+0xdc/0x104) [] (__warn) from [] (warn_slowpath_null+0x28/0x30) r9:00d8 r8:bf2d9e00 r7:0001 r6:c5b5b010 r5:c5b5b014 r4:c7b2b8a0 [] (warn_slowpath_null) from [] (cppi41_stop_chan+0x248/0x338 [cppi41]) [] (cppi41_stop_chan [cppi41]) from [] (cppi41_dma_channel_abort+0x118/0x290 [musb_hdrc]) r7:c5848010 r6:0002 r5:0001 r4:c7077454 [] (cppi41_dma_channel_abort [musb_hdrc]) from [] (musb_gadget_disable+0xfc/0x240 [musb_hdrc]) r10:c7077000 r9:fee00410 r8:2093 r7:c5848010 r6:bf2d9de4 r5:c5848010 r4:c5848410 [] (musb_gadget_disable [musb_hdrc]) from [] (usb_ep_disable+0x30/0x3c [udc_core]) r10: r9:c71440
Re: [RESEND PATCH v5] ARM: dts: da850: Add the CPPI 4.1 DMA to the USB OTG controller
On Thursday 25 May 2017 02:41 PM, Sekhar Nori wrote: > On Friday 19 May 2017 07:03 PM, Alexandre Bailon wrote: >> This adds the CPPI 4.1 DMA controller to the USB OTG controller. >> >> Changes since v2: >> - Fixed the the property reg-names (had glue register defined) >> - Removed few useless property >> >> Signed-off-by: Alexandre Bailon > > Looks good to me. Will wait some more time for other feedback before > applying. Applied to v4.13/dt Thanks, Sekhar -- 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: [RESEND PATCH v5] ARM: dts: da850: Add the CPPI 4.1 DMA to the USB OTG controller
On Friday 19 May 2017 07:03 PM, Alexandre Bailon wrote: > This adds the CPPI 4.1 DMA controller to the USB OTG controller. > > Changes since v2: > - Fixed the the property reg-names (had glue register defined) > - Removed few useless property > > Signed-off-by: Alexandre Bailon Looks good to me. Will wait some more time for other feedback before applying. Thanks, Sekhar -- 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] usb: gadget: f_uac2: calculate wMaxPacketSize before endpoint match
Calculate wMaxPacketSize before endpoint matching the
descriptor is found.
This allows audio gadget to be used with controllers
which have a shortage or unavailability of endpoints
that can handle max packet size of 1023 (FS) or 1024
(HS).
With this audio gadget can be used on TI's OMAP-L138 SoC
which has a MUSB HS controller with endpoints having max
packet size much less than 1023 or 1024. See mode_2_cfg in
drivers/usb/musb/musb_core.c
Signed-off-by: Sekhar Nori
---
drivers/usb/gadget/function/f_uac2.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index f6a0d3a1311b..5a7ba058d947 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -1065,6 +1065,12 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
agdev->as_in_intf = ret;
agdev->as_in_alt = 0;
+ /* Calculate wMaxPacketSize according to audio bandwidth */
+ set_ep_max_packet_size(uac2_opts, &fs_epin_desc, 1000, true);
+ set_ep_max_packet_size(uac2_opts, &fs_epout_desc, 1000, false);
+ set_ep_max_packet_size(uac2_opts, &hs_epin_desc, 8000, true);
+ set_ep_max_packet_size(uac2_opts, &hs_epout_desc, 8000, false);
+
agdev->out_ep = usb_ep_autoconfig(gadget, &fs_epout_desc);
if (!agdev->out_ep) {
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
@@ -1080,12 +1086,6 @@ afunc_bind(struct usb_configuration *cfg, struct
usb_function *fn)
uac2->p_prm.uac2 = uac2;
uac2->c_prm.uac2 = uac2;
- /* Calculate wMaxPacketSize according to audio bandwidth */
- set_ep_max_packet_size(uac2_opts, &fs_epin_desc, 1000, true);
- set_ep_max_packet_size(uac2_opts, &fs_epout_desc, 1000, false);
- set_ep_max_packet_size(uac2_opts, &hs_epin_desc, 8000, true);
- set_ep_max_packet_size(uac2_opts, &hs_epout_desc, 8000, false);
-
hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
--
2.9.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: [PATCH] usb: musb: musb_cppi41: Update an error message
On Tuesday 02 May 2017 06:24 PM, Bin Liu wrote:
> On Tue, May 02, 2017 at 11:21:54AM +0530, Sekhar Nori wrote:
>> Hi Alexandre,
>>
>> On Friday 28 April 2017 09:34 PM, Alexandre Bailon wrote:
>>> If dma_request_slave_channel() failed to return a channel,
>>> then the driver will print an error and request to defer probe.
>>> Update the error message to explain we are defrering probe.
>>>
>>> Signed-off-by: Alexandre Bailon
>>> ---
>>> drivers/usb/musb/musb_cppi41.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
>>> index e7c8b1b..e6b9161 100644
>>> --- a/drivers/usb/musb/musb_cppi41.c
>>> +++ b/drivers/usb/musb/musb_cppi41.c
>>> @@ -676,6 +676,7 @@ static int cppi41_dma_controller_start(struct
>>> cppi41_dma_controller *controller)
>>> dc = dma_request_slave_channel(dev->parent, str);
>>> if (!dc) {
>>> dev_err(dev, "Failed to request %s.\n", str);
>>> + dev_info(dev, "Deferring probe.\n");
>>> ret = -EPROBE_DEFER;
>>> goto err;
>>> }
>>
>> It looks like you will be better-off using dma_request_chan() instead so
>> you can get an error pointer back and not return -EPROBE_DEFER
>> irrespective of the actual error.
>
> I thought about it, but -EPROBE_DEFER is the only error code returned in
> dma_request_chan(), so nothing else to distinguish here, improving the
> dev_err message would be an simpler fix.
The kernel-doc comment for dma_request_chan() says it can return any
error pointer and not just -EPROBE_DEFER. So rather than depending on
the current implementation which might change couple of kernel versions
later, we should assume any error pointer can be returned.
Thanks,
Sekhar
--
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] usb: musb: musb_cppi41: Update an error message
Hi Alexandre,
On Friday 28 April 2017 09:34 PM, Alexandre Bailon wrote:
> If dma_request_slave_channel() failed to return a channel,
> then the driver will print an error and request to defer probe.
> Update the error message to explain we are defrering probe.
>
> Signed-off-by: Alexandre Bailon
> ---
> drivers/usb/musb/musb_cppi41.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
> index e7c8b1b..e6b9161 100644
> --- a/drivers/usb/musb/musb_cppi41.c
> +++ b/drivers/usb/musb/musb_cppi41.c
> @@ -676,6 +676,7 @@ static int cppi41_dma_controller_start(struct
> cppi41_dma_controller *controller)
> dc = dma_request_slave_channel(dev->parent, str);
> if (!dc) {
> dev_err(dev, "Failed to request %s.\n", str);
> + dev_info(dev, "Deferring probe.\n");
> ret = -EPROBE_DEFER;
> goto err;
> }
It looks like you will be better-off using dma_request_chan() instead so
you can get an error pointer back and not return -EPROBE_DEFER
irrespective of the actual error.
Thanks,
Sekhar
--
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 v2] ARM: davinci: Add the clock for the CPPI 4.1 DMA engine
On Friday 07 April 2017 11:01 PM, Alexandre Bailon wrote:
>
>
> On 04/07/2017 06:15 PM, Alexandre Bailon wrote:
>>
>>
>> On 04/07/2017 04:36 PM, Sekhar Nori wrote:
>>> On Wednesday 05 April 2017 10:47 PM, Alexandre Bailon wrote:
>>>> The CPPI 4.1 DMA is sharing its clock with the USB OTG,
>>>> and most of the time, the clock will be enabled by USB.
>>>> But during the init of the DMA, USB is not enabled (waiting for DMA),
>>>> and then we must enable the clock before doing anything.
>>>> Add the clock for the CPPI 4.1 DMA engine.
>>>>
>>>> Signed-off-by: Alexandre Bailon
>>>> ---
>>>> arch/arm/mach-davinci/da830.c | 6 ++
>>>> arch/arm/mach-davinci/da850.c | 6 ++
>>>> 2 files changed, 12 insertions(+)
>>>>
>>>> diff --git a/arch/arm/mach-davinci/da830.c
>>>> b/arch/arm/mach-davinci/da830.c
>>>> index 073c458..bd88470 100644
>>>> --- a/arch/arm/mach-davinci/da830.c
>>>> +++ b/arch/arm/mach-davinci/da830.c
>>>> @@ -304,6 +304,11 @@ static struct clk usb20_clk = {
>>>> .gpsc= 1,
>>>> };
>>>> +static struct clk cppi41_clk = {
>>>> +.name= "cppi41",
>>>> +.parent= &usb20_clk,
>>>> +};
>>>> +
>>>> static struct clk aemif_clk = {
>>>> .name= "aemif",
>>>> .parent= &pll0_sysclk3,
>>>> @@ -413,6 +418,7 @@ static struct clk_lookup da830_clks[] = {
>>>> CLK("davinci-mcasp.1",NULL,&mcasp1_clk),
>>>> CLK("davinci-mcasp.2",NULL,&mcasp2_clk),
>>>> CLK("musb-da8xx","usb20",&usb20_clk),
>>>> +CLK("cppi41-dmaengine",NULL,&cppi41_clk),
>>> I dont see this device name being used in current linux-next. Is this
>>> name accepted ?
>> There is here a typo. The name should be cppi41-dma-engine.
>> I will fix it.
> Actually, it is not a typo. It would have be more logical to name it
> cppi41-dma-engine
> (like the driver name) but the name is correct.
> The device name is not yet in linux-next as the device is created in
> da8xx driver.
> http://marc.info/?l=linux-usb&m=149080474124498&w=2
Alright, applied this and sent a pull request. Made some minor changes
to commit message text.
Thanks,
Sekhar
--
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] usb: dwc3: gadget: delay unmap of bounced requests
Hi Felipe,
On Monday 10 April 2017 05:53 PM, Felipe Balbi wrote:
>
> Hi,
>
> Roger Quadros writes:
>>> From: Janusz Dziedzic
>>>
>>> In the case of bounced ep0 requests, we must delay DMA operation until
>>> after ->complete() otherwise we might overwrite contents of req->buf.
>>>
>>> This caused problems with RNDIS gadget.
>>>
>>> Signed-off-by: Janusz Dziedzic
>>> Signed-off-by: Felipe Balbi
>>> ---
>>
>> Sekhar found out that this fixes RNDIS for us on v4.9.
>> Could you please tag this for stable? Thanks.
>
> Was this a regression on v4.9 or was it already broken? Was it broken
> forever or which commit regressed it? If it falls into
> "has-never-worked-before" then I'm not sure we should tag stable. If we
> can blame a commit, then I can send the patch to stable, no issues.
Mainline v4.9 works. v4.9.20 is broken because commit d62145929992
("usb: dwc3: gadget: always unmap EP0 requests") was backported it.
I think the $SUBJECT patch needs to be propagated to all the stable
kernels to which commit d62145929992 was back-ported to.
Thanks,
Sekhar
--
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 v2] ARM: davinci: Add the clock for the CPPI 4.1 DMA engine
On Wednesday 05 April 2017 10:47 PM, Alexandre Bailon wrote:
> The CPPI 4.1 DMA is sharing its clock with the USB OTG,
> and most of the time, the clock will be enabled by USB.
> But during the init of the DMA, USB is not enabled (waiting for DMA),
> and then we must enable the clock before doing anything.
> Add the clock for the CPPI 4.1 DMA engine.
>
> Signed-off-by: Alexandre Bailon
> ---
> arch/arm/mach-davinci/da830.c | 6 ++
> arch/arm/mach-davinci/da850.c | 6 ++
> 2 files changed, 12 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
> index 073c458..bd88470 100644
> --- a/arch/arm/mach-davinci/da830.c
> +++ b/arch/arm/mach-davinci/da830.c
> @@ -304,6 +304,11 @@ static struct clk usb20_clk = {
> .gpsc = 1,
> };
>
> +static struct clk cppi41_clk = {
> + .name = "cppi41",
> + .parent = &usb20_clk,
> +};
> +
> static struct clk aemif_clk = {
> .name = "aemif",
> .parent = &pll0_sysclk3,
> @@ -413,6 +418,7 @@ static struct clk_lookup da830_clks[] = {
> CLK("davinci-mcasp.1", NULL, &mcasp1_clk),
> CLK("davinci-mcasp.2", NULL, &mcasp2_clk),
> CLK("musb-da8xx", "usb20",&usb20_clk),
> + CLK("cppi41-dmaengine", NULL, &cppi41_clk),
I dont see this device name being used in current linux-next. Is this
name accepted ?
The patch otherwise looks okay.
Thanks,
Sekhar
--
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] ARM: davinci: Add the clock for the CPPI 4.1 DMA engine
On Wednesday 29 March 2017 09:39 PM, Alexandre Bailon wrote:
> The CPPI 4.1 DMA is sharing its clock with the USB OTG,
> and most of the time, the clock will be enabled by USB.
> But during the init of the DMA, USB is not enabled (waiting for DMA),
> and then we must enable the clock before to do anything.
> Add the clock for the CPPI 4.1 DMA engine.
>
> Note:
> This patch is to apply instead of:
> "ARM: davinci: Make the usb20 clock available to PM runtime"
Okay, but I still liked the fact that that patch was using NULL as
con_id for MUSB clock. That makes sense because MUSB gets a single clock
input. I think you should still make that change. If not for v4.12, then
for v4.13.
>
> Signed-off-by: Alexandre Bailon
> ---
> arch/arm/mach-davinci/da830.c| 1 +
> arch/arm/mach-davinci/da850.c| 1 +
> arch/arm/mach-davinci/da8xx-dt.c | 2 ++
> 3 files changed, 4 insertions(+)
>
> diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
> index 073c458..ae4a8a5 100644
> --- a/arch/arm/mach-davinci/da830.c
> +++ b/arch/arm/mach-davinci/da830.c
> @@ -413,6 +413,7 @@ static struct clk_lookup da830_clks[] = {
> CLK("davinci-mcasp.1", NULL, &mcasp1_clk),
> CLK("davinci-mcasp.2", NULL, &mcasp2_clk),
> CLK("musb-da8xx", "usb20",&usb20_clk),
> + CLK("cppi41-dmaengine", NULL, &usb20_clk),
Did you try reading /sys/kernel/debug/davinci_clocks after this patch?
It will hang because of the loop created here.
Looks like what you want is cppi4.1 dma clock to be a child of MUSB
clock. That way, even if DMA is enabled before MUSB, it still works.
Thanks,
Sekhar
--
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] usb: hcd: use correct device pointer for dma ops
commit a8c06e407ef9 ("usb: separate out sysdev pointer from
usb_bus") converted to use hcd->self.sysdev for DMA
operations instead of hcd->self.controller but forgot to do
it for one instance.
This gets caught when DMA debugging is enabled since dma map
and unmap end up using different device pointers.
Fix it.
Fixes: a8c06e407ef9 ("usb: separate out sysdev pointer from usb_bus")
Reported-by: Carlos Hernandez
Acked-by: Roger Quadros
Signed-off-by: Sekhar Nori
---
drivers/usb/core/hcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index da7ee5735c14..49550790a3cb 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1436,7 +1436,7 @@ void usb_hcd_unmap_urb_setup_for_dma(struct usb_hcd *hcd,
struct urb *urb)
{
if (IS_ENABLED(CONFIG_HAS_DMA) &&
(urb->transfer_flags & URB_SETUP_MAP_SINGLE))
- dma_unmap_single(hcd->self.controller,
+ dma_unmap_single(hcd->self.sysdev,
urb->setup_dma,
sizeof(struct usb_ctrlrequest),
DMA_TO_DEVICE);
--
2.9.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: [RESEND PATCH v6 4/4] usb: musb: da8xx: Add a primary support of PM runtime
On Tuesday 28 March 2017 06:09 AM, Kevin Hilman wrote:
> Alexandre Bailon writes:
>
>> Currently, MUSB DA8xx glue driver doesn't have PM runtime support.
>> Because the CPPI 4.1 is using the same clock as MUSB DA8xx and
>> CPPI 4.1 is a child of MUSB DA8xx glue, add support of PM runtime
>> to the DA8xx glue driver in order to let the CPPI 4.1 driver manage
>> the clock by using PM runtime.
>>
>> Signed-off-by: Alexandre Bailon
>
> Dumb question: even if the clock is shared with cppi4, doesn't the
> use-couting in the clock API handle it so that things should function
> fine even without this patch?
>
> Some other comments/questions below...
>
>> ---
>> drivers/usb/musb/da8xx.c | 27 ---
>> 1 file changed, 8 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>> index ed28afd..89e12f6 100644
>> --- a/drivers/usb/musb/da8xx.c
>> +++ b/drivers/usb/musb/da8xx.c
>> @@ -30,7 +30,6 @@
>> */
>>
>> #include
>> -#include
>> #include
>> #include
>> #include
>> @@ -86,7 +85,6 @@ struct da8xx_glue {
>> struct device *dev;
>> struct platform_device *musb;
>> struct platform_device *usb_phy;
>> -struct clk *clk;
>> struct phy *phy;
>> };
>>
>> @@ -376,11 +374,7 @@ static int da8xx_musb_init(struct musb *musb)
>>
>> musb->mregs += DA8XX_MENTOR_CORE_OFFSET;
>>
>> -ret = clk_prepare_enable(glue->clk);
>> -if (ret) {
>> -dev_err(glue->dev, "failed to enable clock\n");
>> -return ret;
>> -}
>> +pm_runtime_get_sync(musb->controller->parent);
>>
>> /* Returns zero if e.g. not clocked */
>> rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG);
>> @@ -423,7 +417,7 @@ static int da8xx_musb_init(struct musb *musb)
>> err_phy_power_on:
>> phy_exit(glue->phy);
>> fail:
>> -clk_disable_unprepare(glue->clk);
>> +pm_runtime_put(musb->controller->parent);
>> return ret;
>> }
>>
>> @@ -435,7 +429,7 @@ static int da8xx_musb_exit(struct musb *musb)
>>
>> phy_power_off(glue->phy);
>> phy_exit(glue->phy);
>> -clk_disable_unprepare(glue->clk);
>> +pm_runtime_put(musb->controller->parent);
>>
>> usb_put_phy(musb->xceiv);
>>
>> @@ -519,7 +513,6 @@ static int da8xx_probe(struct platform_device *pdev)
>> struct musb_hdrc_platform_data *pdata = dev_get_platdata(&pdev->dev);
>> struct da8xx_glue *glue;
>> struct platform_device_info pinfo;
>> -struct clk *clk;
>> struct device_node *np = pdev->dev.of_node;
>> int ret;
>>
>> @@ -527,12 +520,6 @@ static int da8xx_probe(struct platform_device *pdev)
>> if (!glue)
>> return -ENOMEM;
>>
>> -clk = devm_clk_get(&pdev->dev, "usb20");
>> -if (IS_ERR(clk)) {
>> -dev_err(&pdev->dev, "failed to get clock\n");
>> -return PTR_ERR(clk);
>> -}
>
> Something isn't quite right here.
>
> This clk_get uses a con_id "usb20", but when converting to runtime PM,
> we rely on the pm_clk layer (used on davinci for runtime PM) to do clock
> lookups by the default con_ids. I guess this still probably works
> because we fallback to the NULL con_id.
Right, since DaVinci uses pm_clk layer for runtime PM, we support
runtime PM on only two types of clocks. Those with con_id in the list of
con_ids recognized by pm_clk layer or those with NULL con_id. For
devices which use a single clock, NULL con_id is preferred.
Thanks,
Sekhar
--
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 v2] arm: davinci: Make the usb20 clock available to PM runtime
On Tuesday 24 January 2017 04:02 PM, Alexandre Bailon wrote: > Add usb20 to the list of clock supported by PM runtime. Patch description does not match contents. You should rather talk about: since USB20 subsystem uses just one clock, there is no need of a con_id, so we are using NULL. Also, "ARM: " prefix should be capitalized . > > Signed-off-by: Alexandre Bailon > Suggested-by: Sekhar Nori Thanks, Sekhar -- 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] arm: davinci: Make the usb20 clock available to PM runtime
On Thursday 19 January 2017 11:01 PM, Alexandre Bailon wrote:
> On 01/19/2017 05:49 PM, Grygorii Strashko wrote:
>> On 01/19/2017 09:08 AM, Alexandre Bailon wrote:
>>> On 01/19/2017 03:48 PM, Sekhar Nori wrote:
>>>> On Thursday 19 January 2017 07:39 PM, Alexandre Bailon wrote:
>>>>> Add usb20 to the list of clock supported by PM runtime.
>>>>>
>>>>> Signed-off-by: Alexandre Bailon
>>>>> ---
>>>>> arch/arm/mach-davinci/pm_domain.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-davinci/pm_domain.c
>>>>> b/arch/arm/mach-davinci/pm_domain.c
>>>>> index 78eac2c..66471f2 100644
>>>>> --- a/arch/arm/mach-davinci/pm_domain.c
>>>>> +++ b/arch/arm/mach-davinci/pm_domain.c
>>>>> @@ -23,7 +23,7 @@ static struct dev_pm_domain davinci_pm_domain = {
>>>>>
>>>>> static struct pm_clk_notifier_block platform_bus_notifier = {
>>>>> .pm_domain = &davinci_pm_domain,
>>>>> - .con_ids = { "fck", "master", "slave", NULL },
>>>>> + .con_ids = { "fck", "master", "slave", "usb20", NULL },
>>>>
>>>> Instead of doing this, can we drop the con_id from musb clock? Looking
>>>> at the USB clocking diagram in the TRM. There is a single clock input to
>>>> the USB 2.0 subsystem. There is no real need for a con_id at all.
>>> Currently, the con_id is required to get the usb20 clock from usb-da8xx.c
>>> I will try to figure out which changes are required remove con_id.
>>
>> It most probably should be renamed to "fck" then it should work with your
>> patch "[PATCH v3 5/5] usb: musb: da8xx: Add a primary support of PM runtime".
> Actually, because of the USB phy, more changes are required.
> Something like that works for me.
There are too many things going on in your patch and I am not clear why
all of those changes are needed. The following diff worked for me on top
of master branch of my tree. Can you check if this works for you? If not,
I need some more explanation of why you are making all the changes that
you are.
Thanks,
Sekhar
---8<---
diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
index 073c458d0c67..2cfd9d70a818 100644
--- a/arch/arm/mach-davinci/da830.c
+++ b/arch/arm/mach-davinci/da830.c
@@ -412,7 +412,7 @@ static struct clk_lookup da830_clks[] = {
CLK("davinci-mcasp.0", NULL, &mcasp0_clk),
CLK("davinci-mcasp.1", NULL, &mcasp1_clk),
CLK("davinci-mcasp.2", NULL, &mcasp2_clk),
- CLK("musb-da8xx", "usb20",&usb20_clk),
+ CLK("musb-da8xx", NULL, &usb20_clk),
CLK(NULL, "aemif",&aemif_clk),
CLK(NULL, "aintc",&aintc_clk),
CLK(NULL, "secu_mgr", &secu_mgr_clk),
diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
index 55f6e1172517..946cd06d8c05 100644
--- a/arch/arm/mach-davinci/da850.c
+++ b/arch/arm/mach-davinci/da850.c
@@ -567,7 +567,7 @@ static struct clk_lookup da850_clks[] = {
*/
CLK(NULL, "aemif",&aemif_nand_clk),
CLK("ohci-da8xx", "usb11",&usb11_clk),
- CLK("musb-da8xx", "usb20",&usb20_clk),
+ CLK("musb-da8xx", NULL, &usb20_clk),
CLK("spi_davinci.0",NULL, &spi0_clk),
CLK("spi_davinci.1",NULL, &spi1_clk),
CLK("vpif", NULL, &vpif_clk),
diff --git a/arch/arm/mach-davinci/usb-da8xx.c
b/arch/arm/mach-davinci/usb-da8xx.c
index 9a6af0bd5dc3..9b667689b665 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -275,7 +275,7 @@ int __init da8xx_register_usb20_phy_clk(bool
use_usb_refclkin)
struct clk *parent;
int ret;
- usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+ usb20_clk = clk_get(&da8xx_usb20_dev.dev, NULL);
ret = PTR_ERR_OR_ZERO(usb20_clk);
if (ret)
return ret;
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index e89708d839e5..8f6f0efd978e 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -502,7 +502,7 @@ static int da8xx_probe(struct platform_device *pdev)
if (!glue)
return -ENOMEM;
- clk = devm_clk_get(&pdev->dev, "usb20");
+ clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(clk)) {
dev_err(&pdev->dev, "failed to get clock\n");
return PTR_ERR(clk);
--
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 5/5] usb: musb: da8xx: Add a primary support of PM runtime
On Thursday 19 January 2017 10:12 PM, Sergei Shtylyov wrote: > On 01/19/2017 05:08 PM, Alexandre Bailon wrote: > >> Currently, DA8xx doesn't support PM runtime. >> In addition, the glue driver is managing the clock itself. >> But the CPPI DMA needs to manage this clock too. >> Add support to PM runtime and use the callback to enable / disable >> the clock. > >I think this sentence is stale now And please clarify what you mean by "DA8xx doesn't support PM runtime." because we do have drivers using PM runtime being used on DA8xx. Thanks, Sekhar -- 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] arm: davinci: Make the usb20 clock available to PM runtime
On Thursday 19 January 2017 07:39 PM, Alexandre Bailon wrote:
> Add usb20 to the list of clock supported by PM runtime.
>
> Signed-off-by: Alexandre Bailon
> ---
> arch/arm/mach-davinci/pm_domain.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-davinci/pm_domain.c
> b/arch/arm/mach-davinci/pm_domain.c
> index 78eac2c..66471f2 100644
> --- a/arch/arm/mach-davinci/pm_domain.c
> +++ b/arch/arm/mach-davinci/pm_domain.c
> @@ -23,7 +23,7 @@ static struct dev_pm_domain davinci_pm_domain = {
>
> static struct pm_clk_notifier_block platform_bus_notifier = {
> .pm_domain = &davinci_pm_domain,
> - .con_ids = { "fck", "master", "slave", NULL },
> + .con_ids = { "fck", "master", "slave", "usb20", NULL },
Instead of doing this, can we drop the con_id from musb clock? Looking
at the USB clocking diagram in the TRM. There is a single clock input to
the USB 2.0 subsystem. There is no real need for a con_id at all.
Thanks,
Sekhar
--
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 1/3] ARM: dts: da850: Add the cppi41 dma node
On Tuesday 10 January 2017 07:15 PM, Alexandre Bailon wrote: > On 01/09/2017 07:26 PM, Sergei Shtylyov wrote: >>> +0x201000 0x1000 >>> +0x202000 0x1000 >>> +0x204000 0x4000>; >>> +reg-names = "glue", "controller", >>> +"scheduler", "queuemgr"; >>> +interrupts = <58>; >>> +interrupt-names = "glue"; >>> +#dma-cells = <2>; >>> +#dma-channels = <4>; >>> +#dma-requests = <256>; >>> +status = "disabled"; >> >>Why disabled? It doesn't use any external pins... > Will fix it. Please keep it disabled. See the other thread on this. Thanks, Sekhar -- 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 08/11] dmaengine: cppi41: Implement the glue for da8xx
On Tuesday 10 January 2017 09:19 PM, Tony Lindgren wrote: > * Alexandre Bailon [170110 07:23]: >> On 01/10/2017 11:05 AM, Sekhar Nori wrote: >>> On DA8xx, CPPI 4.1 DMAengine is not an independent system resource, but >>> embedded within the USB 2.0 controller. So, I think all that is needed >>> is for MUSB DA8xx glue to trigger probe of CPPI 4.1 dmaengine driver >>> when it is ready. I am not sure all this DA850-specific clock handling >>> is really necessary. >> Actually, we have a circular dependency. >> USB core tries to get DMA channels during the probe, which fails because >> CPPI 4.1 driver is not ready. >> But it will never be ready because the USB clock must be enabled before >> DMA driver probe, what will not happen because USB driver have disabled >> the clock when probe failed. >> >> Someone in the office suggested me to use the component API, >> that could help me to probe the DMA from the USB probe. >> >> Another way to workaround the dependency would be to do defer the >> function calls that access to hardware to avoid to control clock from >> DMA driver. > > Or you really have some wrapper IP block around musb and cppi41 just > like am335x has. > > See drivers/usb/musb/musb_am335x.c and compatible properties for > "ti,am33xx-usb" and it's children in am33xx.dtsi. This looks like what will will need to da850 too. Alexandre, If the wrapper is going to work, can you see if it will end up breaking DT compatibility once introduced? If yes, I suggest reverting the usb1 DT node in da850-lcdk.dts so we don't get into a DT compatibility problem once v4.10 releases. Thanks, Sekhar -- 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 08/11] dmaengine: cppi41: Implement the glue for da8xx
On Tuesday 10 January 2017 03:08 PM, Alexandre Bailon wrote:
> On 01/09/2017 07:08 PM, Grygorii Strashko wrote:
>>
>>
>> On 01/09/2017 10:06 AM, Alexandre Bailon wrote:
>>> The da8xx has a cppi41 dma controller.
>>> This is add the glue layer required to make it work on da8xx,
>>> as well some changes in driver (e.g to manage clock).
>>>
>>> Signed-off-by: Alexandre Bailon
>>> ---
>>> drivers/dma/cppi41.c | 95
>>>
>>> 1 file changed, 95 insertions(+)
>>>
>>> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
>>> index 939398e..4318e53 100644
>>> --- a/drivers/dma/cppi41.c
>>> +++ b/drivers/dma/cppi41.c
>>> @@ -1,3 +1,4 @@
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -86,10 +87,19 @@
>>>
>>> #define USBSS_IRQ_PD_COMP (1 << 2)
>>>
>>> +/* USB DA8XX */
>>> +#define DA8XX_INTR_SRC_MASKED 0x38
>>> +#define DA8XX_END_OF_INTR 0x3c
>>> +
>>> +#define DA8XX_QMGR_PENDING_MASK(0xf << 24)
>>> +
>>> +
>>> +
>>> /* Packet Descriptor */
>>> #define PD2_ZERO_LENGTH(1 << 19)
>>>
>>> #define AM335X_CPPI41 0
>>> +#define DA8XX_CPPI41 1
>>>
>>> struct cppi41_channel {
>>> struct dma_chan chan;
>>> @@ -158,6 +168,9 @@ struct cppi41_dd {
>>>
>>> /* context for suspend/resume */
>>> unsigned int dma_tdfdq;
>>> +
>>> + /* da8xx clock */
>>> + struct clk *clk;
>>> };
>>>
>>> static struct chan_queues am335x_usb_queues_tx[] = {
>>> @@ -232,6 +245,20 @@ static const struct chan_queues am335x_usb_queues_rx[]
>>> = {
>>> [29] = { .submit = 30, .complete = 155},
>>> };
>>>
>>> +static const struct chan_queues da8xx_usb_queues_tx[] = {
>>> + [0] = { .submit = 16, .complete = 24},
>>> + [1] = { .submit = 18, .complete = 24},
>>> + [2] = { .submit = 20, .complete = 24},
>>> + [3] = { .submit = 22, .complete = 24},
>>> +};
>>> +
>>> +static const struct chan_queues da8xx_usb_queues_rx[] = {
>>> + [0] = { .submit = 1, .complete = 26},
>>> + [1] = { .submit = 3, .complete = 26},
>>> + [2] = { .submit = 5, .complete = 26},
>>> + [3] = { .submit = 7, .complete = 26},
>>> +};
>>> +
>>> struct cppi_glue_infos {
>>> irqreturn_t (*isr)(int irq, void *data);
>>> const struct chan_queues *queues_rx;
>>> @@ -366,6 +393,26 @@ static irqreturn_t am335x_cppi41_irq(int irq, void
>>> *data)
>>> return cppi41_irq(cdd);
>>> }
>>>
>>> +static irqreturn_t da8xx_cppi41_irq(int irq, void *data)
>>> +{
>>> + struct cppi41_dd *cdd = data;
>>> + u32 status;
>>> + u32 usbss_status;
>>> +
>>> + status = cppi_readl(cdd->qmgr_mem + QMGR_PEND(0));
>>> + if (status & DA8XX_QMGR_PENDING_MASK)
>>> + cppi41_irq(cdd);
>>> + else
>>> + return IRQ_NONE;
>>> +
>>> + /* Re-assert IRQ if there no usb core interrupts pending */
>>> + usbss_status = cppi_readl(cdd->usbss_mem + DA8XX_INTR_SRC_MASKED);
>>> + if (!usbss_status)
>>> + cppi_writel(0, cdd->usbss_mem + DA8XX_END_OF_INTR);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> static dma_cookie_t cppi41_tx_submit(struct dma_async_tx_descriptor *tx)
>>> {
>>> dma_cookie_t cookie;
>>> @@ -972,8 +1019,19 @@ static const struct cppi_glue_infos am335x_usb_infos
>>> = {
>>> .platform = AM335X_CPPI41,
>>> };
>>>
>>> +static const struct cppi_glue_infos da8xx_usb_infos = {
>>> + .isr = da8xx_cppi41_irq,
>>> + .queues_rx = da8xx_usb_queues_rx,
>>> + .queues_tx = da8xx_usb_queues_tx,
>>> + .td_queue = { .submit = 31, .complete = 0 },
>>> + .first_completion_queue = 24,
>>> + .qmgr_num_pend = 2,
>>> + .platform = DA8XX_CPPI41,
>>> +};
>>> +
>>> static const struct of_device_id cppi41_dma_ids[] = {
>>> { .compatible = "ti,am3359-cppi41", .data = &am335x_usb_infos},
>>> + { .compatible = "ti,da8xx-cppi41", .data = &da8xx_usb_infos},
>>> {},
>>> };
>>> MODULE_DEVICE_TABLE(of, cppi41_dma_ids);
>>> @@ -995,6 +1053,13 @@ static int is_am335x_cppi41(struct device *dev)
>>> return cdd->platform == AM335X_CPPI41;
>>> }
>>>
>>> +static int is_da8xx_cppi41(struct device *dev)
>>> +{
>>> + struct cppi41_dd *cdd = dev_get_drvdata(dev);
>>> +
>>> + return cdd->platform == DA8XX_CPPI41;
>>> +}
>>> +
>>> #define CPPI41_DMA_BUSWIDTHS (BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
>>> BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
>>> BIT(DMA_SLAVE_BUSWIDTH_3_BYTES) | \
>>> @@ -1058,6 +1123,21 @@ static int cppi41_dma_probe(struct platform_device
>>> *pdev)
>>> cdd->first_completion_queue = glue_info->first_completion_queue;
>>> cdd->platform = glue_info->platform;
>>>
>>> + if (is_da8xx_cppi41(dev)) {
>>> + cdd->clk = devm_clk_get(&pdev->dev, "usb20");
>>> + ret = PTR_ERR_OR_ZERO(cdd->clk);
>>> + if (ret) {
>>> + dev_err(&pdev->dev, "failed to get clock\n");
>>> + goto err_clk_en;
>>> + }
>>> +
>>> + r
Re: [PATCH 2/3] usb: musb: Add a quirk to preserve MUSB_DEVCTL during suspend
On Monday 05 December 2016 10:17 PM, Bin Liu wrote: > On Fri, Dec 02, 2016 at 03:23:50PM +0100, Alexandre Bailon wrote: >> On 11/29/2016 06:56 PM, Bin Liu wrote: >>> On Tue, Nov 29, 2016 at 06:41:04PM +0100, Alexandre Bailon wrote: On 11/29/2016 05:16 PM, Bin Liu wrote: > Hi, > > On Mon, Nov 28, 2016 at 05:26:20PM +0100, Alexandre Bailon wrote: >> On da8xx, VBUS is not maintained during suspend when musb is in host >> mode. >> On resume, all the connected devices will be disconnected and then will >> be enumerated again. >> This happens because MUSB_DEVCTL is cleared during suspend. >> Add a quirk to preserve MUSB_DEVCTL during a suspend. > > Will preserving MUSB_DEVCTL prevent the device getting disconnected? Yes. Preserving MUSB_DEVCTL keeps VBUS on though the PHY is off during suspend. >>> >>> VBUS will be on, but does it prevent disconnecting the usb device on >>> resume? I don't have a DA8xx to test, but I doubt it, since the PHY is >>> off. >> Actually, the PHY is not really off. > > I guess the PHY should be off when the system is suspended for an > aggressive power saving. > > Sekhar, any comments? No, nothing from my side. I haven't really played with this side of DA850 at all. For your reference, this is what chapter 10.11.1 referenced by Alexandre says: " The USB modules can be clock gated using the PSC; however, this does not power down/clock gate the PHY logic. You can put the USB2.0 PHY and OTG module in the lowest power state, when not in use, by writing to the USB0PHYPWDN and the USB0OTGPWRDN bits in the Chip Configuration 2 Register (CFGCHIP2) in the System Configuration (SYSCFG) Module chapter. " It is little bit ambiguous on if USB0PHYPWDN and USB0OTGPWRDN do really power down the PHY. I would just program the bits suggested by the manual and assume that the hardware reaches the safest low power state it can reach. Thanks, Sekhar -- 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 v2 1/3] ARM: davinci: da8xx: Fix ohci device name
On Thursday 03 November 2016 09:33 PM, Axel Haslam wrote: > While the clk lookup table is making reference to "ohci" > other subsystems (such as phy) are trying to match "ohci.0" > > Since there is a single ohci instance, instead of changing > the clk name, change the dev id to -1, and add the "-da8xx" > postfix to match the driver name that will also be changed > in a subsequent patch. > > Signed-off-by: Axel Haslam Applied to v4.10/soc Thanks, Sekhar -- 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 10/10] ARM: dts: da850: add usb device node
On Monday 21 November 2016 04:16 PM, Sekhar Nori wrote: >>> In commit 2957e36e76c836b167e5e0c1edb578d8a9bd7af6 in the linux-davinci >>> >> tree, the alias for the musb device is usb0. So, I think we should use >>> >> usb1 >>> >> here instead of ohci - or change the usb0 alias to musb. >>> >> >>> >> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/commit/?h=v4.10/dt&id=2957e36e76c836b167e5e0c1edb578d8a9bd7af6 >> > >> > ok, i will change to usb1, since i will be resubmiting this. > I have already applied a version of this patch. Please re-base against > linux-davinci/master and send a delta patch. Hmm, no. scratch that. I mixed this up with the musb patch I applied. usb1 sounds good. Please also separate out the soc and board specific dts additions for your next version. Thanks, Sekhar -- 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 10/10] ARM: dts: da850: add usb device node
On Monday 21 November 2016 03:57 PM, Axel Haslam wrote:
> On Mon, Nov 21, 2016 at 3:42 AM, David Lechner wrote:
>> On 11/07/2016 02:39 PM, Axel Haslam wrote:
>>>
>>> This adds the ohci device node for the da850 soc.
>>> It also enables it for the omapl138 hawk board.
>>>
>>> Signed-off-by: Axel Haslam
>>> ---
>>> arch/arm/boot/dts/da850-lcdk.dts | 8
>>> arch/arm/boot/dts/da850.dtsi | 8
>>> 2 files changed, 16 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
>>> b/arch/arm/boot/dts/da850-lcdk.dts
>>> index 7b8ab21..aaf533e 100644
>>> --- a/arch/arm/boot/dts/da850-lcdk.dts
>>> +++ b/arch/arm/boot/dts/da850-lcdk.dts
>>> @@ -86,6 +86,14 @@
>>> };
>>> };
>>>
>>> +&usb_phy {
>>> + status = "okay";
>>> +};
>>> +
>>> +&ohci {
>>> + status = "okay";
>>> +};
>>> +
>>> &serial2 {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <&serial2_rxtx_pins>;
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index 2534aab..50e86da 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -405,6 +405,14 @@
>>> >;
>>> status = "disabled";
>>> };
>>> + ohci: usb@0225000 {
>>
>>
>> In commit 2957e36e76c836b167e5e0c1edb578d8a9bd7af6 in the linux-davinci
>> tree, the alias for the musb device is usb0. So, I think we should use usb1
>> here instead of ohci - or change the usb0 alias to musb.
>>
>> https://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/commit/?h=v4.10/dt&id=2957e36e76c836b167e5e0c1edb578d8a9bd7af6
>
> ok, i will change to usb1, since i will be resubmiting this.
I have already applied a version of this patch. Please re-base against
linux-davinci/master and send a delta patch.
Thanks,
Sekhar
--
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 v5 0/2] platform: Add DT support for DA8xx
On Wednesday 16 November 2016 04:37 PM, Alexandre Bailon wrote: > This add and enable the usb otg for da850 and da850-lcdk. > This series depends on "driver: dd DT support for DA8xx" patch set. I see that Bin has already applied this. > If this series is applied before the "usb: musb: da8xx: Fix few issues" patch > set then the usb driver will always retrun -ENODEV. And this seems to be applied too. So I am going ahead and applying this series. For future, please add some information on which device you are referring to in the subject line. Reading the subject line for the cover-letter, there is no way to know what exactly is being worked on here. Thanks, Sekhar -- 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 4/4] ARM: dts: da850: Add the usb otg device node
On Wednesday 16 November 2016 04:05 PM, Alexandre Bailon wrote:
> On 11/15/2016 11:46 AM, Sekhar Nori wrote:
>> On Thursday 03 November 2016 09:29 PM, Alexandre Bailon wrote:
>>> This adds the device tree node for the usb otg
>>> controller present in the da850 family of SoC's.
>>> This also enables the otg usb controller for the lcdk board.
>>>
>>> Signed-off-by: Alexandre Bailon
>>> ---
>>> arch/arm/boot/dts/da850-lcdk.dts | 8
>>> arch/arm/boot/dts/da850.dtsi | 15 +++
>>> 2 files changed, 23 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
>>> b/arch/arm/boot/dts/da850-lcdk.dts
>>> index 7b8ab21..9f5040c 100644
>>> --- a/arch/arm/boot/dts/da850-lcdk.dts
>>> +++ b/arch/arm/boot/dts/da850-lcdk.dts
>>> @@ -158,6 +158,14 @@
>>> rx-num-evt = <32>;
>>> };
>>>
>>> +&usb_phy {
>>> + status = "okay";
>>> + };
>>
>> As mentioned by David already, this node needs to be removed. Please
> I have missed it. But why should I remove it?
> Without it, usb otg won't work.
Grr, I replied to the wrong hunk. The part in da850-lcdk.dts needs to be
preserved (but please fix the indentation on the closing brace).
The part in da850.dtsi needs to be removed as it is already merged.
Thanks,
Sekhar
--
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 4/4] ARM: dts: da850: Add the usb otg device nodeg
On Wednesday 16 November 2016 02:49 AM, Bin Liu wrote:
> On Tue, Nov 15, 2016 at 04:16:02PM +0530, Sekhar Nori wrote:
>> On Thursday 03 November 2016 09:29 PM, Alexandre Bailon wrote:
>>> This adds the device tree node for the usb otg
>>> controller present in the da850 family of SoC's.
>>> This also enables the otg usb controller for the lcdk board.
>>>
>>> Signed-off-by: Alexandre Bailon
>>> ---
>>> arch/arm/boot/dts/da850-lcdk.dts | 8
>>> arch/arm/boot/dts/da850.dtsi | 15 +++
>>> 2 files changed, 23 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
>>> b/arch/arm/boot/dts/da850-lcdk.dts
>>> index 7b8ab21..9f5040c 100644
>>> --- a/arch/arm/boot/dts/da850-lcdk.dts
>>> +++ b/arch/arm/boot/dts/da850-lcdk.dts
>>> @@ -158,6 +158,14 @@
>>> rx-num-evt = <32>;
>>> };
>>>
>>> +&usb_phy {
>>> + status = "okay";
>>> + };
>>
>> As mentioned by David already, this node needs to be removed. Please
>> rebase this on top of latest linux-davinci/master when ready for merging
>> (driver changes accepted).
>
> Dropped this patch due to this comment.
Bin, Please do not apply dts or arch/arm/mach-davinci patches. I have a
bunch queued through my tree and more in pipeline and it will cause
unnecessary merge conflicts in linux-next or at Linus.
For future, I have asked Alexandre to send driver and dts patches as
separate series so there is no confusion on who should apply.
Thanks,
Sekhar
--
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] dma: cpp41: Fix handling of error path
On Wednesday 16 November 2016 02:28 AM, Tony Lindgren wrote:
> * Sekhar Nori [161115 00:35]:
>> If pm_runtime_get_sync() fails due to callback error, then
>> rpm_callback() sets dev.power.runtime_error to an error value which gets
>> cleared by an explicit call to pm_runtime_set_suspended().
>>
>> This will tell the framework that the status of device is suspended.
>> Else, the failure will be sticky and on subsequent attempts,
>> rpm_resume() will keep returning early instead of trying to resume the
>> device again.
>>
>> This is as far as I can gather from code. So, I believe the recovery
>> path should be:
>>
>> if (error < 0) {
>> pm_runtime_set_suspended(cdd->ddev.dev);
>> pm_runtime_put_noidle(cdd->ddev.dev);
>>
>> ...
>
> Well we should test this :)
Yes, right! Was this patch created to fix an error in practice or just
based on code review?
> BTW, what other users of cppi41 do we have in addition to musb? I think
> davinci is or will be using it too?
The list Bin provided seems accurate.
Thanks,
Sekhar
--
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 4/4] ARM: dts: da850: Add the usb otg device node
On Thursday 03 November 2016 09:29 PM, Alexandre Bailon wrote:
> This adds the device tree node for the usb otg
> controller present in the da850 family of SoC's.
> This also enables the otg usb controller for the lcdk board.
>
> Signed-off-by: Alexandre Bailon
> ---
> arch/arm/boot/dts/da850-lcdk.dts | 8
> arch/arm/boot/dts/da850.dtsi | 15 +++
> 2 files changed, 23 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850-lcdk.dts
> b/arch/arm/boot/dts/da850-lcdk.dts
> index 7b8ab21..9f5040c 100644
> --- a/arch/arm/boot/dts/da850-lcdk.dts
> +++ b/arch/arm/boot/dts/da850-lcdk.dts
> @@ -158,6 +158,14 @@
> rx-num-evt = <32>;
> };
>
> +&usb_phy {
> + status = "okay";
> + };
As mentioned by David already, this node needs to be removed. Please
rebase this on top of latest linux-davinci/master when ready for merging
(driver changes accepted).
> +
> +&usb0 {
> + status = "okay";
> +};
> +
> &aemif {
> pinctrl-names = "default";
> pinctrl-0 = <&nand_pins>;
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index f79e1b9..322a31a 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -372,6 +372,21 @@
> >;
> status = "disabled";
> };
> + usb_phy: usb-phy {
> + compatible = "ti,da830-usb-phy";
> + #phy-cells = <1>;
> + status = "disabled";
> + };
> + usb0: usb@20 {
> + compatible = "ti,da830-musb";
> + reg = <0x20 0x1>;
> + interrupts = <58>;
> + interrupt-names = "mc";
> + dr_mode = "otg";
> + phys = <&usb_phy 0>;
> + phy-names = "usb-phy";
> + status = "disabled";
> + };
Can you separate out the soc specific changes from board changes? Please
place the usb0 node above the mdio node. I am trying to get to a rough
ordering based on reg property.
Thanks,
Sekhar
--
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] dma: cpp41: Fix handling of error path
On Saturday 12 November 2016 12:58 AM, Tony Lindgren wrote:
> If we return early on pm_runtime_get() error, we need to also call
> pm_runtime_put_noidle() as pointed out in a musb related thread
> by Johan Hovold . This is to keep the PM runtime
> use counts happy.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: Johan Hovold
> Signed-off-by: Tony Lindgren
> ---
> drivers/dma/cppi41.c | 11 +--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -366,8 +366,11 @@ static int cppi41_dma_alloc_chan_resources(struct
> dma_chan *chan)
> int error;
>
> error = pm_runtime_get_sync(cdd->ddev.dev);
> - if (error < 0)
> + if (error < 0) {
> + pm_runtime_put_noidle(cdd->ddev.dev);
> +
If pm_runtime_get_sync() fails due to callback error, then
rpm_callback() sets dev.power.runtime_error to an error value which gets
cleared by an explicit call to pm_runtime_set_suspended().
This will tell the framework that the status of device is suspended.
Else, the failure will be sticky and on subsequent attempts,
rpm_resume() will keep returning early instead of trying to resume the
device again.
This is as far as I can gather from code. So, I believe the recovery
path should be:
if (error < 0) {
pm_runtime_set_suspended(cdd->ddev.dev);
pm_runtime_put_noidle(cdd->ddev.dev);
...
Thanks,
Sekhar
--
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 2/3] phy: da8xx-usb: rename the ohci device to ohci-da8xx
Hi Kishon, On Thursday 03 November 2016 10:20 PM, Kishon Vijay Abraham I wrote: > > > On Wednesday 02 November 2016 06:14 PM, Axel Haslam wrote: >> There is only one ohci on the da8xx series of chips, >> so remove the ".0" when creating the phy. Also add >> the "-da8xx" postfix to be consistent across davinci >> usb drivers. >> >> Signed-off-by: Axel Haslam > > Acked-by: Kishon Vijay Abraham I You will have to carry this patch from your tree. I thought I can carry the entire series, but the USB patch depends on other patches that Greg has already queued. So I think its best if the individual patches go through their respective trees. Note that there is a v2 already submitted. Thanks, Sekhar -- 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 0/3] fix ohci phy name
On Thursday 03 November 2016 01:54 PM, Axel Haslam wrote: > Hi Sekhar, David, > > It might make sense to have this patch series, > squashed into a single patch, would you agree, > or do you prefer it as is: one-per-subsystem? Patches in the current form are okay. Some coordination is required in getting them merged though. I am happy to take the driver patches through ARM-SoC with ack from respective maintainers. I will need to carry the platform patch through my tree because it conflicts with other changes I have already queued. That said, I am unable to review 3/3 since I am unable to find its baseline. Thanks, Sekhar -- 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 3/3] usb: ohci-da8xx: rename driver to ohci-da8xx
On Wednesday 02 November 2016 06:14 PM, Axel Haslam wrote: > To be consistent on the usb driver for the davinci > platform follow the example of musb, and add the > "-da8xx" postfix to the driver name. > > Signed-off-by: Axel Haslam > --- > drivers/usb/host/ohci-da8xx.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ohci-da8xx.c b/drivers/usb/host/ohci-da8xx.c > index bd6cf3c..b3de8bc 100644 > --- a/drivers/usb/host/ohci-da8xx.c > +++ b/drivers/usb/host/ohci-da8xx.c > @@ -27,7 +27,7 @@ > #include "ohci.h" > > #define DRIVER_DESC "DA8XX" > -#define DRV_NAME "ohci" > +#define DRV_NAME "ohci-da8xx" To which baseline does this patch apply? I don't see this code in linux-next. Thanks, Sekhar -- 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/RFT v2 07/17] ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable
On Tuesday 25 October 2016 09:35 PM, David Lechner wrote:
> On 10/25/2016 05:12 AM, Sekhar Nori wrote:
>> On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote:
>>> diff --git a/arch/arm/mach-davinci/usb-da8xx.c
>>> b/arch/arm/mach-davinci/usb-da8xx.c
>>> index 9e41a7f..982e105 100644
>>> --- a/arch/arm/mach-davinci/usb-da8xx.c
>>> +++ b/arch/arm/mach-davinci/usb-da8xx.c
>>> @@ -53,11 +53,19 @@ int __init da8xx_register_usb_refclkin(int rate)
>>>
>>> static void usb20_phy_clk_enable(struct clk *clk)
>>> {
>>> +struct clk *usb20_clk;
>>> u32 val;
>>> u32 timeout = 50; /* 500 msec */
>>>
>>> val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>>>
>>> +usb20_clk = clk_get(NULL, "usb20");
>>
>> We should not be using a NULL device pointer here. Can you pass the musb
>> device pointer available in the same file? Also, da850_clks[] in da850.c
>> needs to be fixed to add the matching device name.
>
> This clock can be used for usb 1.1 PHY even when musb is not being used,
> so I don't think we can depend on having a musb device here.
Replied to this against the same question in v6 patch series you posted.
> Also, in a previous review, it was decided that the usb clocks should
> *not* be added to da850_clks[] [1]. Instead, they are dynamically
> registered elsewhere.
Thats only the USB phy clocks since there is associated
enable()/disable()/set_parent() code which is better kept outside of
da850.c for readability. No other reason. Lookup for the USB 2.0 module
clock is already present in da850.c
Thanks,
Sekhar
--
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/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
On Tuesday 25 October 2016 09:23 PM, David Lechner wrote:
> Hi Sekhar,
>
> On 10/25/2016 05:17 AM, Sekhar Nori wrote:
>> On Tuesday 25 October 2016 03:07 PM, Axel Haslam wrote:
>>> Hi Sekar,
>>>
>>> On Tue, Oct 25, 2016 at 10:10 AM, Sekhar Nori wrote:
>>>> On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote:
>>>>> From: David Lechner
>>>>>
>>>>> The CFGCHIP registers are used by a number of devices, so using a
>>>>> syscon
>>>>> device to share them. The first consumer of this will by the
>>>>> phy-da8xx-usb
>>>>> driver.
>>>>>
>>>>> Signed-off-by: David Lechner
>>>>> [Axel: minor fix: change id to -1]
>>>>
>>>> Can you please clarify this change? There could be other syscon devices
>>>> on the chip for other common registers. Why use the singular device-id?
>>>>
>>>
>>> in the case of non DT boot, the phy driver is looking for "syscon" :
>>>
>>> d_phy->regmap = syscon_regmap_lookup_by_pdevname("syscon");
>>>
>>> if we register the syscon driver with id = 0, the actual name of the
>>> syscon
>>> device will be "syscon.0" and the phy driver will fail to probe, because
>>> the strncmp match in the syscon driver (syscon_match_pdevname)
>>> will fail.
>>>
>>> should i change the phy driver instead?
>>
>> Yes, please. Forcing only one syscon region for the whole chip will be
>> too restrictive, I am pretty sure.
>>
>> Thanks,
>> Sekhar
>>
>
> In the previous review, you requested that this be changed to -1 [1].
>
> If we change it back to 0, it will also require reverting a patch to the
> phy driver that has already been merged[2].
Sigh. Sorry about going around in circles on this one. Lets go with what
you have. If and when there is a need for another syscon node, the
driver and platform code can be updated. At least we will know why the
change is being done at that time.
Thanks,
Sekhar
--
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/RFT v2 00/17] Add DT support for ohci-da8xx
Hi Axel, On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote: > From: Axel Haslam > > The purpose of this patch series is to add DT support and modernize > the ohci-da8xx glue driver without breaking the non-DT boot, > which is still used in unconverted davinci devices. >From a mach-davinci perspective, there are some patches which seem to be safe to apply and some which depend on corresponding driver changes to get in. In order to speed up the process of applying this series, can you split the mach-davinci updates which are safe to apply into a separate series. For DT patches, the bindings should be accepted. For other patches, they should not be causing any regressions. Thanks, Sekhar -- 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/RFT v2 12/17] USB: ochi-da8xx: Use a regulator for vbus/overcurrent
On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote: > From: Axel Haslam > > Currently, the da8xx ohci driver uses a set of gpios and callbacks in > board files to handle vbus and overcurrent irqs form the power supply. > However, this does not play nice when moving to a DT based boot were > we wont have board files. > > Instead of requesting and handling the gpio, use the regulator framework > to take care of enabling and disabling vbus power. This has the benefit > that we dont need to pass any more platform data to the driver: > > These will be handled by the regulator framework: > set_power -> regulator_enable/regulator_disable > get_power -> regulator_is_enabled > get_oci -> regulator_get_mode > ocic_notify -> regulator notification > > We can keep the default potpgt and use the regulator start delay instead: > potpgt -> regulator startup delay time > > The hawk board does not have a GPIO/OVERCURRENT gpio to control vbus, > (they should not have been decleared/reserved) so, just remove those > definitions from the hwk board file. > > Signed-off-by: Axel Haslam > --- > arch/arm/mach-davinci/board-da830-evm.c | 97 > arch/arm/mach-davinci/board-omapl138-hawk.c | 96 +--- > arch/arm/mach-davinci/include/mach/da8xx.h | 2 +- > arch/arm/mach-davinci/usb-da8xx.c | 3 +- > drivers/usb/host/ohci-da8xx.c | 111 > ++-- > include/linux/platform_data/usb-davinci.h | 19 - > 6 files changed, 105 insertions(+), 223 deletions(-) Can you separate out the driver enhancement from the platform (mach-davinci) changes? They need to go through different trees. Thanks, Sekhar -- 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/RFT v2 08/17] ARM: davinci: hawk: add full constraints for ohci plat boot
On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote: > From: Axel Haslam > > The phy framework requests an optional "phy" regulator. If it does > not find one, it returns -EPROBE_DEFER. In the case of non-DT based boot > for the omap138-lcdk board, this would prevent the usb11 phy to probe > correctly and ohci would not enumerate. > > By calling "regulator_has_full_constraints", An error would be returned nit: prefer regulator_has_full_constraints() > instead of DEFER for the "optional" regulator, and the probe of > the phy driver can continue normally without a regulator. > > Signed-off-by: Axel Haslam Looks good to me. Just drop the "hawk: from subject line since you also touch da830 evm. I am not sure what "ohci plat boot" means. How about the following: "ARM: davinci: da8xx: fix OHCI PHY probe for non-DT boot" Thanks, Sekhar -- 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/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
On Tuesday 25 October 2016 03:07 PM, Axel Haslam wrote:
> Hi Sekar,
>
> On Tue, Oct 25, 2016 at 10:10 AM, Sekhar Nori wrote:
>> On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote:
>>> From: David Lechner
>>>
>>> The CFGCHIP registers are used by a number of devices, so using a syscon
>>> device to share them. The first consumer of this will by the phy-da8xx-usb
>>> driver.
>>>
>>> Signed-off-by: David Lechner
>>> [Axel: minor fix: change id to -1]
>>
>> Can you please clarify this change? There could be other syscon devices
>> on the chip for other common registers. Why use the singular device-id?
>>
>
> in the case of non DT boot, the phy driver is looking for "syscon" :
>
> d_phy->regmap = syscon_regmap_lookup_by_pdevname("syscon");
>
> if we register the syscon driver with id = 0, the actual name of the syscon
> device will be "syscon.0" and the phy driver will fail to probe, because
> the strncmp match in the syscon driver (syscon_match_pdevname)
> will fail.
>
> should i change the phy driver instead?
Yes, please. Forcing only one syscon region for the whole chip will be
too restrictive, I am pretty sure.
Thanks,
Sekhar
--
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/RFT v2 07/17] ARM: davinci: da8xx: Enable the usb20 "per" clk on phy_clk_enable
On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote:
> diff --git a/arch/arm/mach-davinci/usb-da8xx.c
> b/arch/arm/mach-davinci/usb-da8xx.c
> index 9e41a7f..982e105 100644
> --- a/arch/arm/mach-davinci/usb-da8xx.c
> +++ b/arch/arm/mach-davinci/usb-da8xx.c
> @@ -53,11 +53,19 @@ int __init da8xx_register_usb_refclkin(int rate)
>
> static void usb20_phy_clk_enable(struct clk *clk)
> {
> + struct clk *usb20_clk;
> u32 val;
> u32 timeout = 50; /* 500 msec */
>
> val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>
> + usb20_clk = clk_get(NULL, "usb20");
We should not be using a NULL device pointer here. Can you pass the musb
device pointer available in the same file? Also, da850_clks[] in da850.c
needs to be fixed to add the matching device name.
> + if (IS_ERR(usb20_clk)) {
> + pr_err("could not get usb20 clk\n");
> + return;
> + }
Thanks,
Sekhar
--
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/RFT v2 06/17] ARM: davinci: da8xx: Fix some redefined symbol warnings
On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote: > From: Alexandre Bailon > > Some macro for DA8xx CFGCHIP are defined in usb-davinci.h, > but da8xx-cfgchip.h intend to replace them. > The usb-da8xx.c is using both headers, causing redefined symbol warnings. > Remove the old macros. > > Signed-off-by: Alexandre Bailon This is a v4.9-rc bug fix. Can you please post it as a separate patch for Greg to pick up? You can add: Acked-by: Sekhar Nori Thanks, Sekhar -- 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/RFT v2 03/17] ARM: davinci: da8xx: Add USB PHY platform declaration
On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote:
> +static struct platform_device da8xx_usb_phy = {
> + .name = "da8xx-usb-phy",
> + .id = 0,
There is a single phy control in the system for both 1.1 and 2.0 PHYs.
so this can be a singular device (id -1).
Thanks,
Sekhar
--
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/RFT v2 02/17] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
On Monday 24 October 2016 10:16 PM, ahas...@baylibre.com wrote: > From: David Lechner > > The CFGCHIP registers are used by a number of devices, so using a syscon > device to share them. The first consumer of this will by the phy-da8xx-usb > driver. > > Signed-off-by: David Lechner > [Axel: minor fix: change id to -1] Can you please clarify this change? There could be other syscon devices on the chip for other common registers. Why use the singular device-id? > Signed-off-by: Axel Haslam Thanks, Sekhar -- 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 v5 2/5] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
On Wednesday 22 June 2016 10:37 PM, David Lechner wrote:
> On 05/25/2016 06:15 AM, Sekhar Nori wrote:
>> On Tuesday 10 May 2016 10:14 PM, David Lechner wrote:
>>> On 05/10/2016 06:26 AM, Sergei Shtylyov wrote:
>>>> On 5/10/2016 2:46 AM, David Lechner wrote:
[...]
>>>>> +static struct platform_device da8xx_cfgchip_device = {
>>>>> +.name= "syscon",
>>>>> +.id= 0,
>>>>
>>>> Again, -1.
>>>>
>>>> [...]
>>>>
>>>> MBR, Sergei
>>>>
>>>
>>> I wish you would have noticed this when I first submitted it. I remember
>>> going back and forth about this. But it has been too long and I can't
>>> remember the reason why I chose to go this way.
>>>
>>> It seems like changing it broke something with either this one or the
>>> phy device and I opted to keep it this way on both to be consistent. For
>>> example, the USB devices both use id = 0 as well even though there are
>>> only one of each type.
>>
>> Agree with Sergei here. Can you confirm what broke exactly? I think the
>> USB needs to be fixed too.
>>
>> Thanks,
>> Sekhar
>>
>
> I have made the changes from id = 0 to id = -1. I'm not sure what broke
> before, but it is working now.
Thanks!
>
> Re: USB needing fixed, I'm not sure how to do this and I won't have time
> for at least the next 2 or 3 months to do anything about it. The problem
> is that the MUSB can't detect the ID pin. As a workaround, it is
> detecting the VBUS state and using that to determine the ID pin state.
> So, when you attach a self-powered device, it tells the VBUS to turn
> off, which in turn triggers the workaround to say that the ID pin has
> changed state.
I seem to be missing the original problem report. Can you provide a link
to the discussion?
Thanks,
Sekhar
--
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 v5 4/7] phy: da8xx-usb: new driver for DA8xx SoC USB PHY
On Tuesday 10 May 2016 05:10 AM, David Lechner wrote: > This is a new phy driver for the SoC USB controllers on the TI DA8xx > family of microcontrollers. The USB 1.1 PHY is just a simple on/off. > The USB 2.0 PHY also allows overriding the VBUS and ID pins. > > Signed-off-by: David Lechner Reviewed-by: Sekhar Nori Thanks, Sekhar -- 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 v5 0/5] da8xx USB PHY platform devices and clocks (was "da8xx UBS clocks")
On Monday 23 May 2016 08:44 PM, David Lechner wrote: > On 05/09/2016 06:46 PM, David Lechner wrote: >> v5 changes: renamed "usbphy" to "usb_phy" or "usb-phy" as appropriate >> >> David Lechner (5): >>ARM: davinci: da8xx: add usb phy clocks >>ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration. >>ARM: davinci: da8xx: Add USB PHY platform declaration >>ARM: DTS: da850: Add cfgchip syscon node >>ARM: DTS: da850: Add usb phy node >> >> arch/arm/boot/dts/da850.dtsi| 9 ++ >> arch/arm/mach-davinci/board-da830-evm.c | 52 +++ >> arch/arm/mach-davinci/board-da850-evm.c | 4 + >> arch/arm/mach-davinci/board-mityomapl138.c | 4 + >> arch/arm/mach-davinci/board-omapl138-hawk.c | 23 ++- >> arch/arm/mach-davinci/devices-da8xx.c | 28 >> arch/arm/mach-davinci/include/mach/da8xx.h | 6 + >> arch/arm/mach-davinci/usb-da8xx.c | 230 >> +++- >> 8 files changed, 314 insertions(+), 42 deletions(-) >> > > What should I be doing to keep this moving along? We need the related driver changes to be applied first. I could then use an immutable branch to push the platform changes against. I did take a look at the patches and they look good to me. Except the one comment from Sergei which I just now indicated that I agree with. Thanks, Sekhar -- 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 v5 2/5] ARM: davinci: da8xx: Add CFGCHIP syscon platform declaration.
On Tuesday 10 May 2016 10:14 PM, David Lechner wrote:
> On 05/10/2016 06:26 AM, Sergei Shtylyov wrote:
>> On 5/10/2016 2:46 AM, David Lechner wrote:
>>
>>> The CFGCHIP registers are used by a number of devices, so using a syscon
>>> device to share them. The first consumer of this will by the
>>> phy-da8xx-usb
>>> driver.
>>>
>>> Signed-off-by: David Lechner
>> [...]
>>> diff --git a/arch/arm/mach-davinci/devices-da8xx.c
>>> b/arch/arm/mach-davinci/devices-da8xx.c
>>> index 725e693..69d11a1 100644
>>> --- a/arch/arm/mach-davinci/devices-da8xx.c
>>> +++ b/arch/arm/mach-davinci/devices-da8xx.c
>>> @@ -11,6 +11,7 @@
>>> * (at your option) any later version.
>>> */
>>> #include
>>> +#include
>>> #include
>>> #include
>>> #include
>>> @@ -1109,3 +1110,30 @@ int __init da850_register_sata(unsigned long
>>> refclkpn)
>>> return platform_device_register(&da850_sata_device);
>>> }
>>> #endif
>>> +
>>> +static struct syscon_platform_data da8xx_cfgchip_platform_data = {
>>> +.label= "cfgchip",
>>> +};
>>> +
>>> +static struct resource da8xx_cfgchip_resources[] = {
>>> +{
>>> +.start= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP0_REG,
>>> +.end= DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP4_REG + 3,
>>> +.flags= IORESOURCE_MEM,
>>> +},
>>> +};
>>> +
>>> +static struct platform_device da8xx_cfgchip_device = {
>>> +.name= "syscon",
>>> +.id= 0,
>>
>> Again, -1.
>>
>> [...]
>>
>> MBR, Sergei
>>
>
> I wish you would have noticed this when I first submitted it. I remember
> going back and forth about this. But it has been too long and I can't
> remember the reason why I chose to go this way.
>
> It seems like changing it broke something with either this one or the
> phy device and I opted to keep it this way on both to be consistent. For
> example, the USB devices both use id = 0 as well even though there are
> only one of each type.
Agree with Sergei here. Can you confirm what broke exactly? I think the
USB needs to be fixed too.
Thanks,
Sekhar
--
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 00/16] da8xx USB clocks
Hi David, On Thursday 07 April 2016 09:59 PM, David Lechner wrote: > Any further comments before I submit a v4 patchset? Particularly on > patches 3 and 4 which are new in this v3 submission and have not been > commented on yet. I applied some patches which could be applied independently without driver dependency. I have pushed those to my for-testing[1] branch where they will get tested a little more before I send my pull request to ARM-SoC. For your future submissions, you can drop the patches already applied to the branch above. Also, I think it will help if you divide the series into driver and platform changes. You should keep all maintainers copied in both places. But dividing the series this way should help concentrate review. This is a complex series with multiple maintainers involved so it will take some co-ordination. Hopefully it can still all go into the upcoming merge window. Regards, Sekhar [1] http://git.kernel.org/cgit/linux/kernel/git/nsekhar/linux-davinci.git/log/?h=for-testing -- 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 06/16] ARM: davinci: da850: use clk->set_parent for async3
On Friday 25 March 2016 05:21 AM, David Lechner wrote:
> The da850 family of processors has an async3 clock domain that can be
> muxed to either pll0_sysclk2 or pll1_sysclk2. Now that the davinci clocks
> have a set_parent callback, we can use this to control the async3 mux
> instead of a stand-alone function.
>
> This adds a new async3_clk and sets the appropriate child clocks. The
> default is use to pll1_sysclk2 since it is not affected by processor
> frequency scaling.
>
> Signed-off-by: David Lechner
> +static int da850_async3_set_parent(struct clk *clk, struct clk *parent)
> +{
> + u32 val;
> +
> + val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP3_REG));
> +
> + /* Set the Async3 clock domain mux based on the parent clock. */
> + if (parent == &pll0_sysclk2)
> + val &= ~CFGCHIP3_ASYNC3_CLKSRC;
> + else if (parent == &pll1_sysclk2)
> + val |= CFGCHIP3_ASYNC3_CLKSRC;
> + else {
> + pr_err("Bad parent on async3 clock mux.\n");
> + return -EINVAL;
> + }
Since else has braces, need braces on all arm of the if-else construct.
Applied this patch with this fixed locally. checkpatch complains about
this too when --strict option is passed.
Thanks,
Sekhar
--
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 05/16] ARM: davinci: add set_parent callback for mux clocks
On Friday 25 March 2016 05:21 AM, David Lechner wrote:
> Introduce a set_parent callback that will be used for mux clocks, such as
> the USB PHY muxes and the async3 clock domain mux.
>
> Signed-off-by: David Lechner
> ---
>
> v3 changes: none.
>
>
> arch/arm/mach-davinci/clock.c | 17 -
> arch/arm/mach-davinci/clock.h | 1 +
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
> index a5c2629..dfc2eb3 100644
> --- a/arch/arm/mach-davinci/clock.c
> +++ b/arch/arm/mach-davinci/clock.c
> @@ -195,6 +195,13 @@ int clk_set_parent(struct clk *clk, struct clk *parent)
> return -EINVAL;
>
> mutex_lock(&clocks_mutex);
> + if (clk->set_parent) {
> + int ret = clk->set_parent(clk, parent);
Need empty line here.
> + if (ret) {
> + mutex_unlock(&clocks_mutex);
> + return ret;
> + }
> + }
> clk->parent = parent;
> list_del_init(&clk->childnode);
> list_add(&clk->childnode, &clk->parent->children);
> @@ -224,8 +231,16 @@ int clk_register(struct clk *clk)
>
> mutex_lock(&clocks_mutex);
> list_add_tail(&clk->node, &clocks);
> - if (clk->parent)
> + if (clk->parent) {
> + if (clk->set_parent) {
> + int ret = clk->set_parent(clk, clk->parent);
Here too. Applying this patch with these local changes.
BTW, checkpatch complained about these. Please try to address all
warnings unless there is a good reason not to fix one.
Thanks,
Sekhar
--
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 04/16] ARM: davinci: Move clock init after ioremap.
On Friday 25 March 2016 05:21 AM, David Lechner wrote:
> Some clocks (such as the USB PHY clocks in DA8xx) will need to use iomem.
> The davinci_common_init() function must be called before the ioremap, so
> the clock init is now split out as separate function.
>
> Signed-off-by: David Lechner
> ---
>
> v3 changes: This is a new patch. It takes care of the issue of unwanted
> ioremap
> in clock set_parent functions.
>
>
> arch/arm/mach-davinci/clock.c | 4 ++--
> arch/arm/mach-davinci/clock.h | 7 ++-
> arch/arm/mach-davinci/common.c | 6 --
> arch/arm/mach-davinci/da830.c | 2 ++
> arch/arm/mach-davinci/da850.c | 2 ++
> arch/arm/mach-davinci/dm355.c | 1 +
> arch/arm/mach-davinci/dm365.c | 1 +
> arch/arm/mach-davinci/dm644x.c | 1 +
> arch/arm/mach-davinci/dm646x.c | 1 +
> 9 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c
> index 3424eac6..a5c2629 100644
> --- a/arch/arm/mach-davinci/clock.c
> +++ b/arch/arm/mach-davinci/clock.c
> @@ -560,7 +560,7 @@ EXPORT_SYMBOL(davinci_set_pllrate);
> * than that used by default in .c file. The reference clock rate
> * should be updated early in the boot process; ideally soon after the
> * clock tree has been initialized once with the default reference clock
> - * rate (davinci_common_init()).
> + * rate (davinci_clk_init()).
> *
> * Returns 0 on success, error otherwise.
> */
> @@ -581,7 +581,7 @@ int davinci_set_refclk_rate(unsigned long rate)
> return 0;
> }
>
> -int __init davinci_clk_init(struct clk_lookup *clocks)
> +int __init _davinci_clk_init(struct clk_lookup *clocks)
> {
> struct clk_lookup *c;
> struct clk *clk;
> diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h
> index 1e4e836..8b0fbbe 100644
> --- a/arch/arm/mach-davinci/clock.h
> +++ b/arch/arm/mach-davinci/clock.h
> @@ -124,7 +124,12 @@ struct clk {
> .clk = ck, \
> } \
>
> -int davinci_clk_init(struct clk_lookup *clocks);
> +int _davinci_clk_init(struct clk_lookup *clocks);
> +static inline void davinci_clk_init(struct davinci_soc_info *soc_info)
> +{
> + if (soc_info->cpu_clks && _davinci_clk_init(soc_info->cpu_clks))
> + panic("davinci_clk_init: Failed to init clocks.\n");
> +}
I am not sure about the need for an additional API (_davinci_clk_init)
especially when you end up exposing both through clock.h. Just make make
calls from SoC files like:
davinci_clk_init(davinci_soc_info_da830.cpu_clks);
I would ignore the panic() call too since davinci_clk_init() does not
really return an error (never has).
Thanks,
Sekhar
--
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 03/16] ARM: davinici: da8xx: move usb code to new file
On Friday 25 March 2016 05:21 AM, David Lechner wrote:
> We will be adding more da8xx-specific code for phy and clocks, so it will
> be better to have this in a separate file. This way we don't have a bunch
> of #ifdefs for all of the da8xx stuff.
>
> Signed-off-by: David Lechner
Applied this patch with a couple of local changes.
1) The subject line has a typo: s/davinici/davinci
2) There are checkpatch warnings in new code that are coming from old
code. Even though this is just code movement, I prefer new code is free
of checkpatch warnings.
Here is the updated patch.
Thanks,
Sekhar
---8<---
From: David Lechner
Date: Thu, 24 Mar 2016 18:51:28 -0500
Subject: [PATCH] ARM: davinci: da8xx: move usb code to new file
We will be adding more da8xx-specific code for phy and clocks, so it will
be better to have this in a separate file. This way we don't have a bunch
of #ifdefs for all of the da8xx stuff.
While at it, fix some checkpatch warnings coming from existing code.
Signed-off-by: David Lechner
[nsek...@ti.com: typo and checkpatch fixes]
Signed-off-by: Sekhar Nori
---
arch/arm/mach-davinci/Makefile| 4 +-
arch/arm/mach-davinci/usb-da8xx.c | 124 ++
arch/arm/mach-davinci/usb.c | 74 +--
3 files changed, 127 insertions(+), 75 deletions(-)
create mode 100644 arch/arm/mach-davinci/usb-da8xx.c
diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile
index 2e3464b8bab4..da4c336b4637 100644
--- a/arch/arm/mach-davinci/Makefile
+++ b/arch/arm/mach-davinci/Makefile
@@ -14,8 +14,8 @@ obj-$(CONFIG_ARCH_DAVINCI_DM644x) += dm644x.o devices.o
obj-$(CONFIG_ARCH_DAVINCI_DM355)+= dm355.o devices.o
obj-$(CONFIG_ARCH_DAVINCI_DM646x) += dm646x.o devices.o
obj-$(CONFIG_ARCH_DAVINCI_DM365) += dm365.o devices.o
-obj-$(CONFIG_ARCH_DAVINCI_DA830)+= da830.o devices-da8xx.o
-obj-$(CONFIG_ARCH_DAVINCI_DA850)+= da850.o devices-da8xx.o
+obj-$(CONFIG_ARCH_DAVINCI_DA830) += da830.o devices-da8xx.o usb-da8xx.o
+obj-$(CONFIG_ARCH_DAVINCI_DA850) += da850.o devices-da8xx.o usb-da8xx.o
obj-$(CONFIG_AINTC)+= irq.o
obj-$(CONFIG_CP_INTC) += cp_intc.o
diff --git a/arch/arm/mach-davinci/usb-da8xx.c
b/arch/arm/mach-davinci/usb-da8xx.c
new file mode 100644
index ..0cab01f79825
--- /dev/null
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -0,0 +1,124 @@
+/*
+ * DA8xx USB
+ */
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+#define DA8XX_USB0_BASE0x01e0
+#define DA8XX_USB1_BASE0x01e25000
+
+#if IS_ENABLED(CONFIG_USB_MUSB_HDRC)
+
+static struct musb_hdrc_eps_bits musb_eps[] = {
+ { "ep1_tx", 8, },
+ { "ep1_rx", 8, },
+ { "ep2_tx", 8, },
+ { "ep2_rx", 8, },
+ { "ep3_tx", 5, },
+ { "ep3_rx", 5, },
+ { "ep4_tx", 5, },
+ { "ep4_rx", 5, },
+};
+
+static struct musb_hdrc_config musb_config = {
+ .multipoint = true,
+ .dyn_fifo = true,
+ .soft_con = true,
+ .dma= true,
+
+ .num_eps= 5,
+ .dma_channels = 8,
+ .ram_bits = 10,
+ .eps_bits = musb_eps,
+};
+
+static struct musb_hdrc_platform_data usb_data = {
+ /* OTG requires a Mini-AB connector */
+ .mode = MUSB_OTG,
+ .clock = "usb20",
+ .config = &musb_config,
+};
+
+static struct resource da8xx_usb20_resources[] = {
+ {
+ .start = DA8XX_USB0_BASE,
+ .end= DA8XX_USB0_BASE + SZ_64K - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ {
+ .start = IRQ_DA8XX_USB_INT,
+ .flags = IORESOURCE_IRQ,
+ .name = "mc",
+ },
+};
+
+static u64 usb_dmamask = DMA_BIT_MASK(32);
+
+static struct platform_device usb_dev = {
+ .name = "musb-da8xx",
+ .id = -1,
+ .dev = {
+ .platform_data = &usb_data,
+ .dma_mask = &usb_dmamask,
+ .coherent_dma_mask = DMA_BIT_MASK(32),
+ },
+ .resource = da8xx_usb20_resources,
+ .num_resources = ARRAY_SIZE(da8xx_usb20_resources),
+};
+
+int __init da8xx_register_usb20(unsigned int mA, unsigned int potpgt)
+{
+ usb_data.power = mA > 510 ? 255 : mA / 2;
+ usb_data.potpgt = (potpgt + 1) / 2;
+
+ return platform_device_register(&usb_dev);
+}
+
+#else
+
+int __init da8xx_register_usb20(unsigned int mA, unsigned int potpgt)
+{
+ return 0;
+}
+
+#endif /* CONFIG_USB_MUSB_HDRC */
+
+static struct resource da8xx_usb11_resources[] = {
+ [0] = {
+
Re: [PATCH v3 02/16] mfd: da8xx-cfgchip: New header file for CFGCHIP registers.
On Monday 28 March 2016 10:12 PM, Sergei Shtylyov wrote: > Hello. > > On 03/28/2016 06:02 PM, David Lechner wrote: > +/* register offsets */ +#define CFGCHIP_REG(n)(n * 4) +#define CFGCHIP0_REGCFGCHIP_REG(0) +#define CFGCHIP1_REGCFGCHIP_REG(1) +#define CFGCHIP2_REGCFGCHIP_REG(2) +#define CFGCHIP3_REGCFGCHIP_REG(3) +#define CFGCHIP4_REGCFGCHIP_REG(4) >>> >>> Why not just use CFGCHIP_REG(n) directly? >> >> I considered that, but I went this way because A) the TRM uses, for >> example, >> "CFGCHIP2", so I wanted to keep "CFGCHIP" and "2" together IMO, this is not that big of an issue. Anyone reading should be able to make out that CFGCHIP_REG(0) is same as CFGCHIP0 referred to in the TRM. > >I'd just drop the _REG suffix. > >> and B) this tells >> you how many CFGCHIP registers there are, i.e. there is no CFGCHIP5_REG. > >You can tell that in a comment. Having a parametrized macro and using > it to just #define more macros doesn't appeal to me at all... Agree with Sergei, I don't prefer the additional #defines as well. Regards, Sekhar -- 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 01/16] dt: davinci: use proper address after @
On Friday 25 March 2016 05:21 AM, David Lechner wrote: > TI has been using the physical address in DT after the @ in device nodes. > The device tree convention is to use the same address that is used for > the reg property. This updates all davinci DT files to use the proper > convention. > > Signed-off-by: David Lechner > --- > > v3 changes: This is a new patch. Applied through a copy of this patch posted by Petr in thread "ARM: DTS: da850: add node for i2c1" Thanks, Sekhar -- 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 v2 03/11] ARM: davinci: da850: use clk->set_parent for async3
On Thursday 24 March 2016 12:02 AM, David Lechner wrote: > On 03/23/2016 12:29 PM, Sekhar Nori wrote: >> >> Alright, I guess 'can be called' in the comment should have used >> stronger language :) How about late registration of USB clocks as I >> suggested. It should also help consolidate code across da830 and da850. >> > > What about the new async3 clock? It will require later registration too, > but it has many children that depend on it. I guess that was the reason why it was done the way it was. Can we leave that alone for now (not use the new set_parent interface)? You don't really need that to be changed for USB functionality, right? Thanks, sekhar -- 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 v2 04/11] ARM: davinci: da8xx: add usb phy clocks
On Wednesday 23 March 2016 11:15 PM, David Lechner wrote:
> On 03/23/2016 11:56 AM, Sekhar Nori wrote:
>>>
>>> +static struct clk usb_ref_clk = {
>>> +.name= "usb_ref_clk",
>>> +.rate= 4800,
>>> +.set_rate= davinci_simple_set_rate,
>>> +};
>>
>> can we call this usb_refclkin so it matches the TRM name? Also, should
>> this node be not be coming through individual board files as the rate
>> depends on what is connected to the usb_refclkin pin? Or is the
>> expectation that boards will call clk_set_rate() on this clock to the
>> correct value? If yes, I think it is misleading to populate the .rate
>> here.
>
> You are right. When I did this, I was looking at USB 1.1 only, which
> MUST be 48MHz. However, this can be used for USB 2.0 which can accept a
> number of rates.
>
> However, even the main reference oscillator in da850.c has the rate hard
> coded in da850.c (DA850_REF_FREQ).
:)
>
> The clock initialization will fail if a clock does not have a parent or
> a rate, so we have to give it a default rate since it is an external
> clock and has no parent. So, I think 48MHz makes sense for a default
> value. Most boards will probably not be using this clock anyway, but
> rather the PLL in the USB 2.0 PHY.
Alright, I guess the only change is to call it usb_refclkin
>
>
>>> +
>>> +pr_info("Waiting for USB 2.0 PHY clock good...\n");
>>> +while (!(readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG))
>>> +& CFGCHIP2_PHYCLKGD))
>>> +cpu_relax();
>>
>> I guess this is copying some earlier code, but still, it will be nice to
>> see a timeout mechanism here, rather than loop endlessly.
>
> Do you have a suggestion on how to do this?
Simplest would be to use a udelay(1) inside the loop and count a
specific number of times (sufficiently large to not cause false
negatives and sufficiently small so as to not appear that board is
frozen forever).
Thanks,
Sekhar
--
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 v2 03/11] ARM: davinci: da850: use clk->set_parent for async3
On Wednesday 23 March 2016 10:50 PM, David Lechner wrote:
> On 03/23/2016 10:56 AM, Sekhar Nori wrote:
>> On Thursday 17 March 2016 07:56 AM, David Lechner wrote:
>>> The da850 family of processors has an async3 clock domain that can be
>>> muxed to either pll0_sysclk2 or pll1_sysclk2. Now that the davinci
>>> clocks
>>> have a set_parent callback, we can use this to control the async3 mux
>>> instead of a stand-alone function.
>>>
>>> This adds a new async3_clk and sets the appropriate child clocks. The
>>> default is use to pll1_sysclk2 since it is not affected by processor
>>> frequency scaling.
>>>
>>> Signed-off-by: David Lechner
>>> ---
>>
>>> +static int da850_async3_set_parent(struct clk *clk, struct clk *parent)
>>> +{
>>> +u32 __iomem *cfgchip3;
>>> +u32 val;
>>> +
>>> +/*
>>> + * Can't use DA8XX_SYSCFG0_VIRT() here since this can be called
>>> before
>>> + * da8xx_syscfg0_base is initialized.
>>> + */
>>> +cfgchip3 = ioremap(DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP3_REG, 4);
>>
>> Is this just a theoretical possibility or have you seen this happen? I
>> would like to see if there are ways of avoiding this rather than throw
>> away usage of DA8XX_SYSCFG0_VIRT()
>
> Yes, it will not boot without this. The problem comes from the fact that
> clocks are setup in davinci_common_init() which is called before
> da8xx_syscfg0_base = ioremap(DA8XX_SYSCFG0_BASE, SZ_4K) in da850_init()
> (and da830_init()). I also tried moving the ioremap() before
> davinci_common_init(), but davinci_common_init() sets up the iomem, so
> that doesn't work either.
>
> So, if you want to use DA8XX_SYSCFG0_VIRT() here, the clock init would
> have to be split out from davinci_common_init() which would affect all
> davinci devices.
Alright, I guess 'can be called' in the comment should have used
stronger language :) How about late registration of USB clocks as I
suggested. It should also help consolidate code across da830 and da850.
Thanks,
Sekhar
--
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 v2 00/11] da8xx USB clocks
Hi David, On Thursday 17 March 2016 07:09 PM, Sergei Shtylyov wrote: > On 3/17/2016 5:26 AM, David Lechner wrote: > >> OK, ready for round two. > >You're quick... :-) > >> I've added a new callback in the davinci clocks so that they can properly >> handle clock muxing. The clock functions are pretty much the same as >> in the >> previous patch set other than clk_set_parent() now works. >> >> The next new thing is a phy driver for the CFGCHIP2 register that >> controls the >> SoC USB PHY (both USB 1.1 and USB 2.0). The ohci and musb drivers have >> been >> updated to use this new phy driver. > >Fantastic! At least there's some breakthru on this front. +1 Thanks a bunch for resurrecting this code. Regards, Sekhar -- 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 v2 06/11] phy: da8xx-usb: new driver for DA8XX SoC USB PHY
On Thursday 17 March 2016 07:56 AM, David Lechner wrote:
> This is a new phy driver for the SoC USB controllers on the TI DA8XX
> family of microcontrollers. The USB 1.1 PHY is just a simple on/off.
> The USB 2.0 PHY also allows overriding the VBUS and ID pins.
>
> Signed-off-by: David Lechner
> diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
> new file mode 100644
> index 000..93a5f4d
> --- /dev/null
> +++ b/drivers/phy/phy-da8xx-usb.c
> @@ -0,0 +1,295 @@
> +/*
> + * phy-da8xx-usb - TI DaVinci DA8XX USB PHY driver
> + *
> + * Copyright (C) 2016 David Lechner
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* DA8xx CFGCHIP2 (USB PHY Control) register bits */
> +#define PHYCLKGD (1 << 17)
> +#define VBUSSENSE(1 << 16)
> +#define RESET(1 << 15)
> +#define OTGMODE_MASK (3 << 13)
> +#define NO_OVERRIDE (0 << 13)
> +#define FORCE_HOST (1 << 13)
> +#define FORCE_DEVICE (2 << 13)
> +#define FORCE_HOST_VBUS_LOW (3 << 13)
> +#define USB1PHYCLKMUX(1 << 12)
> +#define USB2PHYCLKMUX(1 << 11)
> +#define PHYPWRDN (1 << 10)
> +#define OTGPWRDN (1 << 9)
> +#define DATPOL (1 << 8)
> +#define USB1SUSPENDM (1 << 7)
> +#define PHY_PLLON(1 << 6)
> +#define SESENDEN (1 << 5)
> +#define VBDTCTEN (1 << 4)
> +#define REFFREQ_MASK (0xf << 0)
> +#define REFFREQ_12MHZ(1 << 0)
> +#define REFFREQ_24MHZ(2 << 0)
> +#define REFFREQ_48MHZ(3 << 0)
> +#define REFFREQ_19_2MHZ (4 << 0)
> +#define REFFREQ_38_4MHZ (5 << 0)
> +#define REFFREQ_13MHZ(6 << 0)
> +#define REFFREQ_26MHZ(7 << 0)
> +#define REFFREQ_20MHZ(8 << 0)
> +#define REFFREQ_40MHZ(9 << 0)
Many of these register bits are unused. I guess opinion varies around
this, but I get confused with unnecessary bit definitions and register
offsets. I tend to search for it and its sort of disappointing to see
that its basically unused. Of course, you should wait for PHY
maintainers preference.
> +
> +struct da8xx_usbphy {
> + struct phy_provider *phy_provider;
> + struct phy *usb11_phy;
> + struct phy *usb20_phy;
> + struct clk *usb11_clk;
> + struct clk *usb20_clk;
> + void __iomem*phy_ctrl;
> +};
> +
> +static inline u32 da8xx_usbphy_readl(void __iomem *base)
> +{
> + return readl(base);
> +}
> +
> +static inline void da8xx_usbphy_writel(void __iomem *base, u32 value)
> +{
> + writel(value, base);
> +}
Agree with Sergei that these don't add much value.
> +int da8xx_usb20_phy_set_mode(struct phy *phy, enum musb_mode mode)
> +{
> + struct da8xx_usbphy *d_phy = phy_get_drvdata(phy);
> + u32 val;
> +
> + val = da8xx_usbphy_readl(d_phy->phy_ctrl);
> +
> + val &= ~OTGMODE_MASK;
> + switch (mode) {
> + case MUSB_HOST: /* Force VBUS valid, ID = 0 */
> + val |= FORCE_HOST;
> + break;
> + case MUSB_PERIPHERAL: /* Force VBUS valid, ID = 1 */
> + val |= FORCE_DEVICE;
> + break;
> + case MUSB_OTG: /* Don't override the VBUS/ID comparators */
> + val |= NO_OVERRIDE;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + da8xx_usbphy_writel(d_phy->phy_ctrl, val);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(da8xx_usb20_phy_set_mode);
Hmm, since this driver exports this symbol, I think it should also
provide an include file in include/linux/phy for users of the symbol. Or
perhaps there should be a generic API around this since it looks like
most USB phys will need something similar?
Thanks,
Sekhar
--
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 v2 05/11] dt-bindings: Add bindings for phy-da8xx-usb
On Thursday 17 March 2016 07:56 AM, David Lechner wrote: > Device tree binding for new phy-da8xx-usb driver. > > Signed-off-by: David Lechner > --- > > v2 changes: This is new patch in v2. > > > .../devicetree/bindings/phy/phy-da8xx-usb.txt | 34 > ++ > 1 file changed, 34 insertions(+) > create mode 100644 Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt > > diff --git a/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt > b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt > new file mode 100644 > index 000..ed6b710 > --- /dev/null > +++ b/Documentation/devicetree/bindings/phy/phy-da8xx-usb.txt > @@ -0,0 +1,34 @@ > +TI DaVinci DA8XX USB PHY > + > +Required properties: > + - compatible: must be "ti,da830-usbphy". > + - #phy-cells: must be 1. > + - reg : Address and length of the CFGCHIP2 register. I am not sure passing CFGCHIP2 register as reg property to the phy is future proof. At some point, we do want to move to common clock framework and at that point USB clocks controlled by CFGCHIP2 will be a separate driver needing access to the same register. So I think the CFGCHIP2 access in USB phy driver should happen through a syscon phandle. This needs to happen now, not later since we cannot break DT backward-compatibility. Thanks, Sekhar -- 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 v2 04/11] ARM: davinci: da8xx: add usb phy clocks
On Thursday 17 March 2016 07:56 AM, David Lechner wrote:
> Up to this point, the USB phy clock configuration was handled manually in
> the board files and in the usb drivers. This adds proper clocks so that
> the usb drivers can use clk_get and clk_enable and not have to worry about
> the details. Also, the related code is removed from the board files.
>
> Signed-off-by: David Lechner
> ---
>
> v2 changes: Move clock mux code to set_parent callback. Also fixed some other
> issues from feedback on the previous patch.
>
>
> arch/arm/mach-davinci/board-da830-evm.c | 12 ---
> arch/arm/mach-davinci/board-omapl138-hawk.c | 7 --
> arch/arm/mach-davinci/da830.c | 143
>
> arch/arm/mach-davinci/da850.c | 143
>
> 4 files changed, 286 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-da830-evm.c
> b/arch/arm/mach-davinci/board-da830-evm.c
> index 3d8cf8c..f3a8cc9 100644
> --- a/arch/arm/mach-davinci/board-da830-evm.c
> +++ b/arch/arm/mach-davinci/board-da830-evm.c
> @@ -115,18 +115,6 @@ static __init void da830_evm_usb_init(void)
>*/
> cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>
> - /* USB2.0 PHY reference clock is 24 MHz */
> - cfgchip2 &= ~CFGCHIP2_REFFREQ;
> - cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ;
> -
> - /*
> - * Select internal reference clock for USB 2.0 PHY
> - * and use it as a clock source for USB 1.1 PHY
> - * (this is the default setting anyway).
> - */
> - cfgchip2 &= ~CFGCHIP2_USB1PHYCLKMUX;
> - cfgchip2 |= CFGCHIP2_USB2PHYCLKMUX;
> -
> /*
>* We have to override VBUS/ID signals when MUSB is configured into the
>* host-only mode -- ID pin will float if no cable is connected, so the
> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c
> b/arch/arm/mach-davinci/board-omapl138-hawk.c
> index ee62486..d27e753 100644
> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
> @@ -251,13 +251,6 @@ static __init void omapl138_hawk_usb_init(void)
> return;
> }
>
> - /* Setup the Ref. clock frequency for the HAWK at 24 MHz. */
> -
> - cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> - cfgchip2 &= ~CFGCHIP2_REFFREQ;
> - cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ;
> - __raw_writel(cfgchip2, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> -
> ret = gpio_request_one(DA850_USB1_VBUS_PIN,
> GPIOF_DIR_OUT, "USB1 VBUS");
> if (ret < 0) {
> diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
> index 7187e7f..ee942b0 100644
> --- a/arch/arm/mach-davinci/da830.c
> +++ b/arch/arm/mach-davinci/da830.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -346,6 +347,12 @@ static struct clk i2c1_clk = {
> .gpsc = 1,
> };
>
> +static struct clk usb_ref_clk = {
> + .name = "usb_ref_clk",
> + .rate = 4800,
> + .set_rate = davinci_simple_set_rate,
> +};
can we call this usb_refclkin so it matches the TRM name? Also, should
this node be not be coming through individual board files as the rate
depends on what is connected to the usb_refclkin pin? Or is the
expectation that boards will call clk_set_rate() on this clock to the
correct value? If yes, I think it is misleading to populate the .rate here.
> +
> static struct clk usb11_clk = {
> .name = "usb11",
> .parent = &pll0_sysclk4,
> @@ -353,6 +360,139 @@ static struct clk usb11_clk = {
> .gpsc = 1,
> };
>
> +static void usb20_phy_clk_enable(struct clk *clk)
> +{
> + u32 val;
> +
> + val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> + /*
> + * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
> + * host may use the PLL clock without USB 2.0 OTG being used.
> + */
> + val &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
> + val |= CFGCHIP2_PHY_PLLON;
> +
> + writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> + pr_info("Waiting for USB 2.0 PHY clock good...\n");
> + while (!(readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG))
> + & CFGCHIP2_PHYCLKGD))
> + cpu_relax();
I guess this is copying some earlier code, but still, it will be nice to
see a timeout mechanism here, rather than loop endlessly.
> +}
> +
> +static void usb20_phy_clk_disable(struct clk *clk)
> +{
> + u32 val;
> +
> + val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> + val |= CFGCHIP2_PHYPWRDN;
> + writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +}
> +
> +static int usb20_phy_clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + u32 __iomem *cfgchip2;
> + u32 val;
> +
> + /*
> + * Can't use DA8XX_SYSCFG0_
Re: [PATCH v2 03/11] ARM: davinci: da850: use clk->set_parent for async3
On Thursday 17 March 2016 07:56 AM, David Lechner wrote:
> The da850 family of processors has an async3 clock domain that can be
> muxed to either pll0_sysclk2 or pll1_sysclk2. Now that the davinci clocks
> have a set_parent callback, we can use this to control the async3 mux
> instead of a stand-alone function.
>
> This adds a new async3_clk and sets the appropriate child clocks. The
> default is use to pll1_sysclk2 since it is not affected by processor
> frequency scaling.
>
> Signed-off-by: David Lechner
> ---
> +static int da850_async3_set_parent(struct clk *clk, struct clk *parent)
> +{
> + u32 __iomem *cfgchip3;
> + u32 val;
> +
> + /*
> + * Can't use DA8XX_SYSCFG0_VIRT() here since this can be called before
> + * da8xx_syscfg0_base is initialized.
> + */
> + cfgchip3 = ioremap(DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP3_REG, 4);
Is this just a theoretical possibility or have you seen this happen? I
would like to see if there are ways of avoiding this rather than throw
away usage of DA8XX_SYSCFG0_VIRT()
> + val = readl(cfgchip3);
> +
> + /* Set the USB 1.1 PHY clock mux based on the parent clock. */
Comment is wrong (copy-paste error?)
> + if (parent == &pll0_sysclk2)
> + val &= ~CFGCHIP3_ASYNC3_CLKSRC;
> + else if (parent == &pll1_sysclk2)
> + val |= CFGCHIP3_ASYNC3_CLKSRC;
> + else {
> + pr_err("Bad parent on async3 clock mux.\n");
> + return -EINVAL;
> + }
> +
> + writel(val, cfgchip3);
> +
> + return 0;
> +}
Thanks,
Sekhar
--
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 0/2] AM437x: USB DWC3: save power during system sleep
Hi, This series add support to DWC3 core to conserve power during system sleep by setting the USB DRVVBUS line to a lower power consuming state. Tested to make sure USB host and device works on AM437x with v4.2. Tested for power savings on v4.1 kernel where there is an implementation of suspend- to-RAM for AM437x available. Dave Gerlach (1): ARM: dts: am437x-gp-evm: Add pinctrl states for usb Sekhar Nori (1): usb: dwc3: support for pinctrl state change during system sleep arch/arm/boot/dts/am437x-gp-evm.dts | 30 ++ drivers/usb/dwc3/core.c | 4 2 files changed, 34 insertions(+) -- 2.4.4.408.g16da57c -- 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 1/2] usb: dwc3: support for pinctrl state change during system sleep
Add support for USB DRVVBUS pinctrl state change during suspend/resume. This helps is conserving power during system sleep. Signed-off-by: Sekhar Nori --- drivers/usb/dwc3/core.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index ff5773c66b84..a5ffa66e39fb 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -1093,6 +1093,8 @@ static int dwc3_suspend(struct device *dev) phy_exit(dwc->usb2_generic_phy); phy_exit(dwc->usb3_generic_phy); + pinctrl_pm_select_sleep_state(dev); + return 0; } @@ -1102,6 +1104,8 @@ static int dwc3_resume(struct device *dev) unsigned long flags; int ret; + pinctrl_pm_select_default_state(dev); + usb_phy_init(dwc->usb3_phy); usb_phy_init(dwc->usb2_phy); ret = phy_init(dwc->usb2_generic_phy); -- 2.4.4.408.g16da57c -- 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/2] ARM: dts: am437x-gp-evm: Add pinctrl states for usb
From: Dave Gerlach
Add pinctrl default and sleep states for each usb device.
The only pin that can be controlled is USB_DRVVBUS, this
must be set to MUX_MODE7 (gpio) during sleep to conserve
power.
Signed-off-by: Dave Gerlach
[nsek...@ti.com: move pins to core dwc3]
Signed-off-by: Sekhar Nori
---
arch/arm/boot/dts/am437x-gp-evm.dts | 30 ++
1 file changed, 30 insertions(+)
diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts
b/arch/arm/boot/dts/am437x-gp-evm.dts
index 84aa30c3235a..2e990a5f9e95 100644
--- a/arch/arm/boot/dts/am437x-gp-evm.dts
+++ b/arch/arm/boot/dts/am437x-gp-evm.dts
@@ -409,6 +409,30 @@
0x234 (PIN_OUTPUT_PULLDOWN | MUX_MODE0) /*
uart3_rtsn.uart3_rtsn */
>;
};
+
+ usb1_pins_default: usb1_pins_default {
+ pinctrl-single,pins = <
+ 0x2c0 (DS0_PULL_UP_DOWN_EN | PIN_INPUT_PULLDOWN |
MUX_MODE0)
+ >;
+ };
+
+ usb1_pins_sleep: usb1_pins_sleep {
+ pinctrl-single,pins = <
+ 0x2c0 (DS0_PULL_UP_DOWN_EN | PIN_INPUT_PULLDOWN |
MUX_MODE7)
+ >;
+ };
+
+ usb2_pins_default: usb2_pins_default {
+ pinctrl-single,pins = <
+ 0x2c4 (DS0_PULL_UP_DOWN_EN | PIN_INPUT_PULLDOWN |
MUX_MODE0)
+ >;
+ };
+
+ usb2_pins_sleep: usb2_pins_sleep {
+ pinctrl-single,pins = <
+ 0x2c4 (DS0_PULL_UP_DOWN_EN | PIN_INPUT_PULLDOWN |
MUX_MODE7)
+ >;
+ };
};
&i2c0 {
@@ -615,6 +639,9 @@
&usb1 {
dr_mode = "peripheral";
status = "okay";
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&usb1_pins_default>;
+ pinctrl-1 = <&usb1_pins_sleep>;
};
&usb2_phy2 {
@@ -624,6 +651,9 @@
&usb2 {
dr_mode = "host";
status = "okay";
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&usb2_pins_default>;
+ pinctrl-1 = <&usb2_pins_sleep>;
};
&mac {
--
2.4.4.408.g16da57c
--
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 5/7] ARM: DRA7: hwmod: Add SYSCONFIG for usb_otg_ss
On Wednesday 02 July 2014 04:56 PM, Roger Quadros wrote: > Sekhar, > > On 06/18/2014 02:19 PM, Rajendra Nayak wrote: >> On Wednesday 18 June 2014 04:40 PM, Roger Quadros wrote: >>> + Nishant and Rajendra for review. >>> >>> On 05/05/2014 12:54 PM, Roger Quadros wrote: >>>> Add the sysconfig class bits for the Super Speed USB >>>> controllers >>>> >>>> CC: Paul Walmsley >>>> Signed-off-by: Roger Quadros >> >> verified against TRM version vP, looks good to me. >> Reviewed-by: Rajendra Nayak > > Could you please give your Tested-by tag for this? Then we can take this into > 3.16-rc. Boot tested on my DRA7x EVM. Boot log here: http://paste.ubuntu.com/7741337/ Tested-by: Sekhar Nori Thanks, Sekhar -- 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] USB: OHCI: make ohci-da8xx a separate driver
On 7/2/2013 10:50 PM, Kevin Hilman wrote: > On 07/02/2013 08:14 AM, Manjunath Goudar wrote: >> >> >> On 2 July 2013 20:20, Sergei Shtylyov >> > <mailto:sergei.shtyl...@cogentembedded.com>> wrote: >> >> Hello. >> >> >> On 02-07-2013 15:36, Manjunath Goudar wrote: >> >> Separate the Davinci OHCI host controller driver from ohci-hcd >> host code so that it can be built as a separate driver module. >> This work is part of enabling multi-platform kernels on ARM; >> it would be nice to have in 3.11. >> >> >> One preexisting error: >> "da8xx_syscfg0_base" [drivers/usb/host/ohci-da8xx.__ko] undefined! >> >> >> Fixed eventually using below modification: >> added EXPORT_SYMBOL_GPL(da8xx___syscfg0_base) in >> arch/arm/mach-davinci/devices-__da8xx.c. >> >> >>And you managed to get this fix into the DaVinci tree? I tried it >> long ago and it was refused by then DaVinci maintainer Kevin Hilman. >> >> >> Yes I saw your patch that is what I mentioned in patch description. >> We will wait for DaVinci maintainer response,what he will suggest. > > Note that Sekhar Nori (now Cc'd) is the primary maintainer of davinci, > and I'll defer the final decision to him. However, the mach-davinci > change is not in this patch, so I'm not sure exactly how it relates > here, since that problem exists independently of this patch. > > That being said, I will NAK the above EXPORT_SYMBOL change in > mach-davinci code because passing data between platform code and drivers > via global variables is still a bad idea. Some helper accessor > functions will need to be created to abstract those low-level accesses. Okay, I haven't seen the patch as well, but I agree with Kevin that EXPORT_SYMBOL from platform code is a bad idea and wont help the multi-platform build. Right clean-up for this most probably requires creation of a PHY driver to handle the USB 2.0 and USB 1.1 phy specifics on this chip. Its best to start a mail thread on USB list for guidance. You can keep me in loop too. Thanks, Sekhar -- 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] Generic PHY Framework
Hi Kishon, On 3/20/2013 2:41 PM, Kishon Vijay Abraham I wrote: > Added a generic PHY framework that provides a set of APIs for the PHY drivers > to create/destroy a PHY and APIs for the PHY users to obtain a reference to > the PHY with or without using phandle. To obtain a reference to the PHY > without using phandle, the platform specfic intialization code (say from board > file) should have already called phy_bind with the binding information. The > binding information consists of phy's device name, phy user device name and an > index. The index is used when the same phy user binds to mulitple phys. > > This framework will be of use only to devices that uses external PHY (PHY > functionality is not embedded within the controller). >From a top level what you are doing looks closely related to External connector (extcon). I understand a connector is not the same as a phy, but it will still be useful to know why extcon framework (or some extension of it) will not suffice your needs. You can probably note it in the cover-letter so folks like me get their answer. Thanks, Sekhar -- 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] usb: remove CONFIG_USB_MUSB_HOST etc
On 10/23/2012 6:09 PM, Felipe Balbi wrote: > Hi, > > On Tue, Oct 23, 2012 at 06:04:53PM +0530, Sekhar Nori wrote: >> On 10/8/2012 6:47 PM, Constantine Shulyupin wrote: >>> From: Constantine Shulyupin >>> >>> Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly >>> CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST >>> and set MUSB_OTG configuration by default >>> because this configuration options are removed from Kconfig. >>> >>> Signed-off-by: Constantine Shulyupin >> >> Queuing this patch for v3.8. Since the config options are removed there >> is no use having code which refers to them. The patch has been tested on >> DM644x and DM365 in both host and gadget mode (I will add this >> information to commit text while committing). Without this patch .mode >> seems to be defaulting to MUSB_UNDEFINED which I think is definitely wrong. > > sorry for the delay, this looks ok: > > Acked-by: Felipe Balbi Thanks Felipe, I added your ack. Constantine, Patches touching arch/arm/* should be prefixed with 'ARM:' and those touching mach-davinci should be prefixed with 'davinci:'. I added these two while committing the patch this time. Please take care next time on. Thanks, Sekhar -- 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] usb: remove CONFIG_USB_MUSB_HOST etc
On 10/8/2012 6:47 PM, Constantine Shulyupin wrote:
> From: Constantine Shulyupin
>
> Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
> CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
> and set MUSB_OTG configuration by default
> because this configuration options are removed from Kconfig.
>
> Signed-off-by: Constantine Shulyupin
Queuing this patch for v3.8. Since the config options are removed there
is no use having code which refers to them. The patch has been tested on
DM644x and DM365 in both host and gadget mode (I will add this
information to commit text while committing). Without this patch .mode
seems to be defaulting to MUSB_UNDEFINED which I think is definitely wrong.
Thanks,
Sekhar
>
> ---
> arch/arm/mach-davinci/usb.c |6 --
> 1 file changed, 6 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
> index f77b953..34509ff 100644
> --- a/arch/arm/mach-davinci/usb.c
> +++ b/arch/arm/mach-davinci/usb.c
> @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
> };
>
> static struct musb_hdrc_platform_data usb_data = {
> -#if defined(CONFIG_USB_MUSB_OTG)
> /* OTG requires a Mini-AB connector */
> .mode = MUSB_OTG,
> -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
> - .mode = MUSB_PERIPHERAL,
> -#elif defined(CONFIG_USB_MUSB_HOST)
> - .mode = MUSB_HOST,
> -#endif
> .clock = "usb",
> .config = &musb_config,
> };
>
--
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] usb: remove CONFIG_USB_MUSB_HOST etc
On 10/11/2012 12:05 PM, Heiko Schocher wrote:
> Hello Manjunathappa
>
> On 11.10.2012 07:42, Manjunathappa, Prakash wrote:
>> Hi,
>> On Mon, Oct 08, 2012 at 18:47:07, Constantine Shulyupin wrote:
>>> From: Constantine Shulyupin
>>>
>>> Remove USB configuration in arch/arm/mach-davinci/usb.c accordingly
>>> CONFIG_USB_MUSB_OTG CONFIG_USB_MUSB_PERIPHERAL CONFIG_USB_MUSB_HOST
>>> and set MUSB_OTG configuration by default
>>> because this configuration options are removed from Kconfig.
>>>
>>> Signed-off-by: Constantine Shulyupin
>>>
>>> ---
>>> arch/arm/mach-davinci/usb.c |6 --
>>> 1 file changed, 6 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
>>> index f77b953..34509ff 100644
>>> --- a/arch/arm/mach-davinci/usb.c
>>> +++ b/arch/arm/mach-davinci/usb.c
>>> @@ -42,14 +42,8 @@ static struct musb_hdrc_config musb_config = {
>>> };
>>>
>>> static struct musb_hdrc_platform_data usb_data = {
>>> -#if defined(CONFIG_USB_MUSB_OTG)
>>> /* OTG requires a Mini-AB connector */
>>> .mode = MUSB_OTG,
>>> -#elif defined(CONFIG_USB_MUSB_PERIPHERAL)
>>> -.mode = MUSB_PERIPHERAL,
>>> -#elif defined(CONFIG_USB_MUSB_HOST)
>>> -.mode = MUSB_HOST,
>>> -#endif
>>> .clock= "usb",
>>> .config=&musb_config,
>>> };
>>
>> Tested it on DM6446-EVM for host mode with MSC thumb drive and gadget
>> mode with g-ether. It works.
>>
>> Acked-by: Manjunathappa, Prakash
>
> I sent a similiar patch here:
>
> http://comments.gmane.org/gmane.linux.usb.general/54512
>
> If the issues, mentioned from Sergei for my patch, nullified I add my:
The last outstanding issue from Sergei seems to be additional comments
describing why MUSB_OTG is OK to use.
Prakash/Constantine,
Did you have to make any hardware changes when testing host/gadget on
DM6446 EVM? Or change in kernel configuration?
It appears that there is no way to choose any of the config option
affecting the mode setting. Right now it seems to be just defaulting to
MUSB_UNDEFINED. Setting it to MUSB_OTG would be better than that.
Thanks,
Sekhar
--
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] Enable USB peripheral on dm365 EVM
Hi Constantine,
On 10/4/2012 9:52 PM, Constantine Shulyupin wrote:
> From: Constantine Shulyupin
>
> Signed-off-by: Constantine Shulyupin
> ---
>
> Note:
>
> USBPHY_CTL_PADDR and USBPHY_CLKFREQ_24MHZ are defined in board-dm365-evm.c
> because davinci.h can't be included from drivers/usb/musb/. May be davinci.h
> should be renamed and moved to arch/arm/mach-davinci/include/mach/usb.h like
> arch/arm/plat-omap/include/plat/usb.h
>
> Tested with usb gadget g_zero.
>
> Changelog
>
> Changes since v3 http://www.spinics.net/lists/kernel/msg1412544.html:
> - removed optional altering of pr_info
>
> Changes since v2 http://article.gmane.org/gmane.linux.kernel/1159868/
> - reordered code
> - removed alternation of GPIO33, which is multiplexed with DRVVBUS, because
> is not need for peripheral USB
>
> Changes since v1 http://marc.info/?l=linux-kernel&m=130894150803661&w=2:
> - removed optional code and reordered
>
> This patch is based on code from Arago, Angstom, and RidgeRun projects.
> Original patch by miguel.agui...@ridgerun.com is three years ago:
> -
> http://www.mail-archive.com/davinci-linux-open-source@linux.davincidsp.com/msg14741.html
>
> ---
> arch/arm/mach-davinci/board-dm365-evm.c |8
> arch/arm/mach-davinci/usb.c |2 ++
> drivers/usb/musb/davinci.h |1 +
> drivers/usb/musb/musb_core.c| 20
> 4 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-dm365-evm.c
> b/arch/arm/mach-davinci/board-dm365-evm.c
> index 3a4743b..dfcb67f 100644
> --- a/arch/arm/mach-davinci/board-dm365-evm.c
> +++ b/arch/arm/mach-davinci/board-dm365-evm.c
> @@ -38,6 +38,8 @@
> #include
> #include
> #include
> +#include
> +#include
>
> #include
>
> @@ -92,6 +94,9 @@ static inline int have_tvp7002(void)
> #define CPLD_CCD_DIR3CPLD_OFFSET(0x3f,0)
> #define CPLD_CCD_IO3 CPLD_OFFSET(0x3f,1)
>
> +#define USBPHY_CTL_PADDR 0x01c40034
> +#define USBPHY_CLKFREQ_24MHZ BIT(13)
> +
> static void __iomem *cpld;
>
>
> @@ -613,6 +618,9 @@ static __init void dm365_evm_init(void)
>
> dm365_init_spi0(BIT(0), dm365_evm_spi_info,
> ARRAY_SIZE(dm365_evm_spi_info));
> + writel(readl(IO_ADDRESS(USBPHY_CTL_PADDR)) | USBPHY_CLKFREQ_24MHZ,
> + IO_ADDRESS(USBPHY_CTL_PADDR));
> + davinci_setup_usb(500, 8);
Add a comment here on why current is limited to 500 mA? Probably coming
because of an on-board component?
> }
>
> MACHINE_START(DAVINCI_DM365_EVM, "DaVinci DM365 EVM")
> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
> index 23d2b6d..664c689 100644
> --- a/arch/arm/mach-davinci/usb.c
> +++ b/arch/arm/mach-davinci/usb.c
> @@ -49,6 +49,8 @@ static struct musb_hdrc_platform_data usb_data = {
> .mode = MUSB_PERIPHERAL,
> #elif defined(CONFIG_USB_MUSB_HOST)
> .mode = MUSB_HOST,
> +#else
> + .mode = MUSB_OTG,
This does not look to be directly related to DM365 support and should be
separated into a different patch.
> #endif
> .clock = "usb",
> .config = &musb_config,
> diff --git a/drivers/usb/musb/davinci.h b/drivers/usb/musb/davinci.h
> index 371baa0..e737d97 100644
> --- a/drivers/usb/musb/davinci.h
> +++ b/drivers/usb/musb/davinci.h
> @@ -16,6 +16,7 @@
>
> /* Integrated highspeed/otg PHY */
> #define USBPHY_CTL_PADDR 0x01c40034
> +#define USBPHY_CLKFREQ_24MHZ BIT(13)
You have added this repeat definition and do not use it anywhere.
Looks like all PHY related configuration is currently happening in
drivers/usb/musb/davinci.c and the same register is also being written
to for other platforms. Can you move the code you have included in board
file to the driver? As we move towards DT, we need to avoid register
configurations from board files anyway.
Thanks,
Sekhar
--
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
