Skip to content

Commit

Permalink
USB: host: ehci-atmel: Add support for HSIC phy
Browse files Browse the repository at this point in the history 
Add support for USB Host High Speed Port HSIC phy.

Signed-off-by: Cristian Birsan <cristian.birsan@microchip.com>
  • Loading branch information
@cristibirsan
cristibirsan committed Apr 13, 2021
1 parent fea19cd commit ca368f5
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions drivers/usb/host/ehci-atmel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,18 @@
#include <linux/platform_device.h>
#include <linux/usb.h>
#include <linux/usb/hcd.h>
#include <linux/usb/phy.h>
#include <linux/usb/of.h>

#include "ehci.h"

#define DRIVER_DESC "EHCI Atmel driver"

static const char hcd_name[] = "ehci-atmel";

#define EHCI_INSNREG(index) ((index) * 4 + 0x90)
#define EHCI_INSNREG08_HSIC_EN BIT(2)

This comment has been minimized.

Sign in to view

Copy link
@LeSpocky

LeSpocky Sep 6, 2021 •
edited
Loading

This is AT91 / SAMA5 specific, right? I would propose something like AT91_UHPHS_INSNREG08 and AT91_UHPHS_HSIC_EN instead.

Also that 0x90 literal is somewhat obscure, it does not appear in the SAMA5D2 datasheet.

This comment has been minimized.

Sign in to view

Copy link
@cristibirsan

cristibirsan Sep 6, 2021

Author

This is not necessary AT91/SAMA5 specific, it is related more to the USB Host IP. Similar defines are used also in

linux-at91/drivers/usb/host/ehci-exynos.c

Line 26 in 67a85fd

#define EHCI_INSNREG00(base) (base + 0x90)
Although not intuitive 0x90 is the offset for the vendor registers.

This comment has been minimized.

Sign in to view

Copy link
@LeSpocky

LeSpocky Sep 10, 2021

As far as I can tell the registers defined in linux/usb/ehci_def.h follow the ECHI 1.0 and 1.1 spec. Last common register in the spec is 44h PORTSC. If 90h is really a common base for multiple USB host IPs, then why not add a define with link to source spec or datasheet document? Otherwise that 90h is just an undocumented literal. 🤷🏼

This comment has been minimized.

Sign in to view

Copy link
@cristibirsan

cristibirsan Sep 10, 2021 •
edited
Loading

Author

INSNREG08 has the offset 0xB0. Having in mind that this is the 8th register and the length is 4 bytes, one can compute the base offset like: 0xB0 - 0x20 = 0x90. I can ask for a datasheet update but you have to be patient ... it might take some time until a new version is published.

/* interface and function clocks */
#define hcd_to_atmel_ehci_priv(h) \
((struct atmel_ehci_priv *)hcd_to_ehci(h)->priv)
Expand Down Expand Up @@ -154,6 +159,9 @@ static int ehci_atmel_drv_probe(struct platform_device *pdev)
goto fail_add_hcd;
device_wakeup_enable(hcd->self.controller);

if (of_usb_get_phy_mode(pdev->dev.of_node) == USBPHY_INTERFACE_MODE_HSIC)
writel(EHCI_INSNREG08_HSIC_EN, hcd->regs + EHCI_INSNREG(8));

This comment has been minimized.

Sign in to view

Copy link
@LeSpocky

LeSpocky Sep 6, 2021

  • What about the previous state of the register?
  • Should the flag be reset on driver remove?
  • Other drivers use ehci_writel(), that's probably not needed here, but should it be uniform over different usb host ehci drivers?

This comment has been minimized.

Sign in to view

Copy link
@cristibirsan

cristibirsan Sep 6, 2021

Author

HSIC_EN is the only bit in the UHPHS_INSNREG08 register, other bits are zero out of reset. We do not care about the previous state.

If this mode is used, it won't change when the driver is removed. In the end the hardware connected to this port will still be a HSIC device. Also it can't be unplugged.

The platform for this glue driver is little endian and writel() will do the job. The same is used in the above example already accepted in upstream but ehci_writel() can be also used.

return retval;

fail_add_hcd:
Expand Down

0 comments on commit ca368f5

Please sign in to comment.