Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
On 3/3/2016 2:12 PM, Arnd Bergmann wrote: > On Thursday 03 March 2016 13:52:39 Joao Pinto wrote: >> >> config SCSI_UFS_DWC >> bool >> >> config SCSI_UFS_DWC_TC_PLATFORM >> tristate "DesignWare platform support using a G210 Test Chip" >> depends on SCSI_UFSHCD_PLATFORM >> select SCSI_UFS_DWC >> ---help--- >> Synopsys Test Chip is a PHY for prototyping purposes. >> >> If unsure, say N." >> >> config SCSI_UFS_DWC_TC_PCI >> tristate "DesignWare pci support using a G210 Test Chip" >> depends on SCSI_UFSHCD_PCI >> select SCSI_UFS_DWC >> ---help--- >> Synopsys Test Chip is a PHY for prototyping purposes. >> >> If unsure, say N." >> >> I would keep SCSI_UFS_DWC to avoid building DWC specific source everytime. >> >> Agree? >> > > Yes, looks good to me. > > Arnd > Nice! Thanks for the inputs. I'm going to implement the changes and send a v10 soon. Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
On Thursday 03 March 2016 13:52:39 Joao Pinto wrote: > > config SCSI_UFS_DWC > bool > > config SCSI_UFS_DWC_TC_PLATFORM > tristate "DesignWare platform support using a G210 Test Chip" > depends on SCSI_UFSHCD_PLATFORM > select SCSI_UFS_DWC > ---help--- > Synopsys Test Chip is a PHY for prototyping purposes. > > If unsure, say N." > > config SCSI_UFS_DWC_TC_PCI > tristate "DesignWare pci support using a G210 Test Chip" > depends on SCSI_UFSHCD_PCI > select SCSI_UFS_DWC > ---help--- > Synopsys Test Chip is a PHY for prototyping purposes. > > If unsure, say N." > > I would keep SCSI_UFS_DWC to avoid building DWC specific source everytime. > > Agree? > Yes, looks good to me. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
Hi, On 3/3/2016 12:04 PM, Arnd Bergmann wrote: > On Thursday 03 March 2016 11:39:05 Joao Pinto wrote: >> Hi Arnd, >> >> On 3/2/2016 7:55 PM, Arnd Bergmann wrote: >>> On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote: On 2/19/2016 3:03 PM, Arnd Bergmann wrote: > On Thursday 18 February 2016 17:20:27 Joao Pinto wrote: >> >> Facts: >> >> - Test Chip type are currently not detectable in runtime through the >> controller >> - In the future the Test Chip version will be available in the controller >> - Test Chip initialization is different for each type >> - The IP Core version is 1.40a >> - Test Chip version is 6.00 >> - Teh UFS version is 2.0 > > Ok. > >> Suggested driver architecture: >> >> Platform setup: >> tc-dwc-g210-pltfrm --> tc-dwc-g210 --> ufshcd-dwc-pltfrm --> ufshcd-dwc --> >> ufs >> >> The test chip platform driver could be called through 2 compatibility >> strings. >> indicating the chip's version and bit type: >> "snps, g210-tc-6.00-20bit" >> "snps, g210-tc-6.00-40bit" > > Yes, this sounds good. We can probably skip one of the middle layers, > but basically that is what I was looking for. > >> The device tree node would have additional info compatibility strings as the >> DWC >> IP core version and UFS version: >> "snps, dwc-ufshcd-1.40a" >> "jedec, ufs-2.0" >> >> PCI based setup: >> tc-dwc-g210-pci --> tc-dwc-g210 --> ufshcd-dwc-pci --> ufshcd-dwc --> ufs > > The tc-dwc-g210 portion probably shouldn't depend on both > ufshcd-dwc-pltfrm and ufshcd-dwc-pci here, so how about leaving > those two out? > > > Then it becomes > >tc-dwc-g210-pci ---> tc-dwc-g210 --> ufshcd-dwc --> ufs > tc-dwc-g210-pltfrm --/ Ok, understood. It becomes simpler without the pltfm and pci "middle layer". > >> The test chip type would be configured by a parameter to be passed in the >> kernel >> boot args: tc_type = 20 (20-bit) or tc_type = 40 (40-bit) > > Right. With module_param() helper, this will be either a boot command > line option, or a module load option, depending on whether the driver > is built-on or not. > > modprobe tc-dwc-g210-pci tc_type=20 > > command line: tc-dwc-g210-pci.tc_type=20 > Right, that was the idea. >> Having this in mind the KConfig would be: >> >> "config SCSI_UFS_DWC_HOOKS >> bool > > I think we can now remove the config option for the hooks as well. > >> config SCSI_UFS_DWC_PLAT >> tristate "DesignWare UFS controller platform glue driver" >> depends on SCSI_UFSHCD_PLATFORM >> select SCSI_UFS_DWC_HOOKS >> help >>This selects the DesignWare UFS host controller platform glue driver. >> >>Select this if you have a DesignWare UFS controller on Platform bus. >>If unsure, say N. >> >> config SCSI_UFS_DWC_PCI >> tristate "DesignWare UFS controller pci glue driver" >> depends on SCSI_UFSHCD_PCI >> select SCSI_UFS_DWC_HOOKS >> help >>This selects the DesignWare UFS host controller pci glue driver. >> >>Select this if you have a DesignWare UFS controller on pci bus. >>If unsure, say N. >> >> config SCSI_UFS_DWC_TC >> bool "Support for the Synopsys Test Chip" >> depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) >> ---help--- >>Synopsys Test Chip is a Phy for prototyping purposes. >>This selects the support for the Synopsys Test Chip. >> >>Select this if you have a Synopsys Test Chip. >>If unsure, say N." >> >> Agree with the approach? > > This would work, but I think it's better to define the options in terms > of the top-level drivers, i.e. SCSI_UFS_DWC_TC_PCI and > SCSI_UFS_DWC_TC_PLATFORM, > and then make the other options hidden and implicitly turned out by them. > config SCSI_UFS_DWC bool config SCSI_UFS_DWC_TC_PLATFORM tristate "DesignWare platform support using a G210 Test Chip" depends on SCSI_UFSHCD_PLATFORM select SCSI_UFS_DWC ---help--- Synopsys Test Chip is a PHY for prototyping purposes. If unsure, say N." config SCSI_UFS_DWC_TC_PCI tristate "DesignWare pci support using a G210 Test Chip" depends on SCSI_UFSHCD_PCI select SCSI_UFS_DWC ---help--- Synopsys Test Chip is a PHY for prototyping purposes. If unsure, say N." I would keep SCSI_UFS_DWC to avoid building DWC specific source everytime. Agree? > Arnd > Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
On Thursday 03 March 2016 11:39:05 Joao Pinto wrote: > Hi Arnd, > > On 3/2/2016 7:55 PM, Arnd Bergmann wrote: > > On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote: > >> On 2/19/2016 3:03 PM, Arnd Bergmann wrote: > >>> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote: > > Facts: > > - Test Chip type are currently not detectable in runtime through the > controller > - In the future the Test Chip version will be available in the controller > - Test Chip initialization is different for each type > - The IP Core version is 1.40a > - Test Chip version is 6.00 > - Teh UFS version is 2.0 Ok. > Suggested driver architecture: > > Platform setup: > tc-dwc-g210-pltfrm --> tc-dwc-g210 --> ufshcd-dwc-pltfrm --> ufshcd-dwc --> > ufs > > The test chip platform driver could be called through 2 compatibility strings. > indicating the chip's version and bit type: > "snps, g210-tc-6.00-20bit" > "snps, g210-tc-6.00-40bit" Yes, this sounds good. We can probably skip one of the middle layers, but basically that is what I was looking for. > The device tree node would have additional info compatibility strings as the > DWC > IP core version and UFS version: > "snps, dwc-ufshcd-1.40a" > "jedec, ufs-2.0" > > PCI based setup: > tc-dwc-g210-pci --> tc-dwc-g210 --> ufshcd-dwc-pci --> ufshcd-dwc --> ufs The tc-dwc-g210 portion probably shouldn't depend on both ufshcd-dwc-pltfrm and ufshcd-dwc-pci here, so how about leaving those two out? Then it becomes tc-dwc-g210-pci ---> tc-dwc-g210 --> ufshcd-dwc --> ufs tc-dwc-g210-pltfrm --/ > The test chip type would be configured by a parameter to be passed in the > kernel > boot args: tc_type = 20 (20-bit) or tc_type = 40 (40-bit) Right. With module_param() helper, this will be either a boot command line option, or a module load option, depending on whether the driver is built-on or not. modprobe tc-dwc-g210-pci tc_type=20 command line: tc-dwc-g210-pci.tc_type=20 > Having this in mind the KConfig would be: > > "config SCSI_UFS_DWC_HOOKS > bool I think we can now remove the config option for the hooks as well. > config SCSI_UFS_DWC_PLAT > tristate "DesignWare UFS controller platform glue driver" > depends on SCSI_UFSHCD_PLATFORM > select SCSI_UFS_DWC_HOOKS > help > This selects the DesignWare UFS host controller platform glue driver. > > Select this if you have a DesignWare UFS controller on Platform bus. > If unsure, say N. > > config SCSI_UFS_DWC_PCI > tristate "DesignWare UFS controller pci glue driver" > depends on SCSI_UFSHCD_PCI > select SCSI_UFS_DWC_HOOKS > help > This selects the DesignWare UFS host controller pci glue driver. > > Select this if you have a DesignWare UFS controller on pci bus. > If unsure, say N. > > config SCSI_UFS_DWC_TC > bool "Support for the Synopsys Test Chip" > depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) > ---help--- > Synopsys Test Chip is a Phy for prototyping purposes. > This selects the support for the Synopsys Test Chip. > > Select this if you have a Synopsys Test Chip. > If unsure, say N." > > Agree with the approach? This would work, but I think it's better to define the options in terms of the top-level drivers, i.e. SCSI_UFS_DWC_TC_PCI and SCSI_UFS_DWC_TC_PLATFORM, and then make the other options hidden and implicitly turned out by them. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
Hi Arnd, On 3/2/2016 7:55 PM, Arnd Bergmann wrote: > On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote: >> On 2/19/2016 3:03 PM, Arnd Bergmann wrote: >>> On Thursday 18 February 2016 17:20:27 Joao Pinto wrote: Facts: - Test Chip type are currently not detectable in runtime through the controller - In the future the Test Chip version will be available in the controller - Test Chip initialization is different for each type - The IP Core version is 1.40a - Test Chip version is 6.00 - Teh UFS version is 2.0 Suggested driver architecture: Platform setup: tc-dwc-g210-pltfrm --> tc-dwc-g210 --> ufshcd-dwc-pltfrm --> ufshcd-dwc --> ufs The test chip platform driver could be called through 2 compatibility strings. indicating the chip's version and bit type: "snps, g210-tc-6.00-20bit" "snps, g210-tc-6.00-40bit" The device tree node would have additional info compatibility strings as the DWC IP core version and UFS version: "snps, dwc-ufshcd-1.40a" "jedec, ufs-2.0" PCI based setup: tc-dwc-g210-pci --> tc-dwc-g210 --> ufshcd-dwc-pci --> ufshcd-dwc --> ufs The test chip type would be configured by a parameter to be passed in the kernel boot args: tc_type = 20 (20-bit) or tc_type = 40 (40-bit) Having this in mind the KConfig would be: "config SCSI_UFS_DWC_HOOKS bool config SCSI_UFS_DWC_PLAT tristate "DesignWare UFS controller platform glue driver" depends on SCSI_UFSHCD_PLATFORM select SCSI_UFS_DWC_HOOKS help This selects the DesignWare UFS host controller platform glue driver. Select this if you have a DesignWare UFS controller on Platform bus. If unsure, say N. config SCSI_UFS_DWC_PCI tristate "DesignWare UFS controller pci glue driver" depends on SCSI_UFSHCD_PCI select SCSI_UFS_DWC_HOOKS help This selects the DesignWare UFS host controller pci glue driver. Select this if you have a DesignWare UFS controller on pci bus. If unsure, say N. config SCSI_UFS_DWC_TC bool "Support for the Synopsys Test Chip" depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) ---help--- Synopsys Test Chip is a Phy for prototyping purposes. This selects the support for the Synopsys Test Chip. Select this if you have a Synopsys Test Chip. If unsure, say N." Agree with the approach? Thanks for the help. Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
On Wednesday 02 March 2016 16:46:47 Joao Pinto wrote: > On 2/19/2016 3:03 PM, Arnd Bergmann wrote: > > On Thursday 18 February 2016 17:20:27 Joao Pinto wrote: > >> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > >> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt > > Please for the last time (!) add a proper version number of the > > specific IP block to the compatible strings so you can identify > > what hardware you are talking to. > > > > You earlier confused the version number of the UFS standard with > > the version number of the controller, and that has been clarified > > now, but you really still need to use a version for the hardware > > as well. > > Ok, we can have a "snps,ufshcd-dwc-1.1" and a "snps,ufshcd-dwc-2.0". Agree? I have no idea what versions of the dwc hardware block exist, but I find it highly suspicious that the numbers happen to be the same as the UFS protocol numbers, so I'd say that is probably not the version of the IP block. There are a few things you could try to find out the actual version: * If you are able to contact the team that worked on the test chip, please ask them what hardware revision you have. * if you have some form of documentation of the hardware, look on the first few pages of the manual that describes the registers to see if the document has a revision history. * If you have access to the hardware design files, look at the file names. On https://www.synopsys.com/dw/ipdir.php?ds=ufs, I see a version "1.30a" listed, which follows the typical numbering scheme that your employer uses, a single digit followed by a dot and a two-digit number and then a letter. There is also a test chip with version 1.10a listed on the same page, and that follows the same numbering system. See if you can find a version number that fits into that scheme in the documents you have available. > >> +config SCSI_UFS_DWC_TC > >> + bool "Support for the Synopsys Test Chip" > >> + depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) > >> + ---help--- > >> +Synopsys Test Chip is a Phy for prototyping purposes. > >> +This selects the support for the Synopsys Test Chip. > >> + > >> +Select this if you have a Synopsys Test Chip. > >> +If unsure, say N. > >> + > >> +config SCSI_UFS_DWC_20BIT_RMMI > >> + bool "20-bit RMMI support" > >> + depends on SCSI_UFS_DWC_TC > >> + ---help--- > >> +This specifies that the Synopsys Test Chip supports 20-bit RMMI. > >> + > >> +Select this if you are using a 20-bit RMMI Synopsys Test Chip. > >> +If unsure, say N. > >> + > >> +config SCSI_UFS_DWC_40BIT_RMMI > >> + bool "40-bit RMMI support" > >> + depends on SCSI_UFS_DWC_TC > >> + ---help--- > >> +Synopsys Test Chip is a Phy for prototyping purposes. > >> + > >> +Select this if you are using a 40-bit RMMI Synopsys Test Chip. > >> +If unsure, say N. > > > > I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI > > and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always > > support both. There is not really much need for the options > > as this is just a test chip, and nobody is going to care much > > about saving a few bytes of object code. > > We need to know if we are dealing with a 20-bit test chip or a 40-bit test > chip > because the initialization is different. That can be made in the device tree > as > you say bellow, but we can also use a setup in which the host is a PC (so no > device tree) connected by pci bus to an fpga containing the UFS controller. > Having this, I think that the only way is to choose the 20/40bit stuff in the > menuconfig. NAK. Mutually exclusive compile-time configuration options are always wrong. If the PCI hosts both have the same PCI device ID and there is no other identification register that lets you find out which one you have, maybe you can use a module parameter that defaults to invalid and that has to be set explicitly when loading the PCI driver? Are both test chips the same way? I see that the driver supports two distinct PCI device IDs, so please check of they both come in variations for the two PHYs, or if at least one of them always uses the same PHY so you don't need the module parameter for that. > >> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile > >> index 8303bcc..bab8c05 100644 > >> --- a/drivers/scsi/ufs/Makefile > >> +++ b/drivers/scsi/ufs/Makefile > >> @@ -1,4 +1,7 @@ > >> # UFSHCD makefile > >> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o > >> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o > >> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o > >> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o > >> obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o > >> obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o > > > > However, please split out the SCSI_UFS_DWC_TC specific bits into > > a separate file, and put the module_platform_driver() bits into > > that file, to get the right abstraction where the most specific > >
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
Hi Arnd, On 2/19/2016 3:03 PM, Arnd Bergmann wrote: > On Thursday 18 February 2016 17:20:27 Joao Pinto wrote: >> >> Documentation/devicetree/bindings/ufs/ufs-dwc.txt | 19 + >> MAINTAINERS | 6 + >> drivers/scsi/ufs/Kconfig | 51 +++ >> drivers/scsi/ufs/Makefile | 3 + >> drivers/scsi/ufs/ufs-dwc-pci.c| 172 + >> drivers/scsi/ufs/ufs-dwc.c| 96 + >> drivers/scsi/ufs/ufshcd-dwc.c | 431 >> ++ >> drivers/scsi/ufs/ufshcd-dwc.h | 18 + >> drivers/scsi/ufs/ufshci-dwc.h | 42 +++ >> drivers/scsi/ufs/unipro.h | 39 ++ > > I still think this can be split up further. From your previous > explanation, I understand that there is a specific test chip > that uses the "snps,ufshcd-dwc" implementation along with some > other glue logic. > > Please split this out so you have anything related to the test > chips separate from the patch that implements core logic. Ok, I will split more the patches. > >> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt >> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt >> new file mode 100644 >> index 000..59e9822 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt >> @@ -0,0 +1,19 @@ >> +* Universal Flash Storage (UFS) DesignWare Host Controller >> + >> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers. >> +Each UFS controller instance should have its own node. >> + >> +Required properties: >> +- compatible: compatible list must contain "snps,ufshcd-dwc" and >> should >> + also contain the JEDEC version of the controller: >> +"jedec,ufs-1.1" >> +"jedec,ufs-2.0" >> +- reg : >> +- interrupts: > > > Based on your last reply to me (sorry for coming back to this so > late), I think having just "snps,ufshcd-dwc" as the compatible > string is not appropriate: This assumes that all "snps,ufshcd-dwc" > instances are 100% compatible, but your code makes it very clear > that this is not the case. > > Please for the last time (!) add a proper version number of the > specific IP block to the compatible strings so you can identify > what hardware you are talking to. > > You earlier confused the version number of the UFS standard with > the version number of the controller, and that has been clarified > now, but you really still need to use a version for the hardware > as well. Ok, we can have a "snps,ufshcd-dwc-1.1" and a "snps,ufshcd-dwc-2.0". Agree? > >> +config SCSI_UFS_DWC_TC >> +bool "Support for the Synopsys Test Chip" >> +depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT) >> +---help--- >> + Synopsys Test Chip is a Phy for prototyping purposes. >> + This selects the support for the Synopsys Test Chip. >> + >> + Select this if you have a Synopsys Test Chip. >> + If unsure, say N. >> + >> +config SCSI_UFS_DWC_20BIT_RMMI >> +bool "20-bit RMMI support" >> +depends on SCSI_UFS_DWC_TC >> +---help--- >> + This specifies that the Synopsys Test Chip supports 20-bit RMMI. >> + >> + Select this if you are using a 20-bit RMMI Synopsys Test Chip. >> + If unsure, say N. >> + >> +config SCSI_UFS_DWC_40BIT_RMMI >> +bool "40-bit RMMI support" >> +depends on SCSI_UFS_DWC_TC >> +---help--- >> + Synopsys Test Chip is a Phy for prototyping purposes. >> + >> + Select this if you are using a 40-bit RMMI Synopsys Test Chip. >> + If unsure, say N. > > I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI > and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always > support both. There is not really much need for the options > as this is just a test chip, and nobody is going to care much > about saving a few bytes of object code. We need to know if we are dealing with a 20-bit test chip or a 40-bit test chip because the initialization is different. That can be made in the device tree as you say bellow, but we can also use a setup in which the host is a PC (so no device tree) connected by pci bus to an fpga containing the UFS controller. Having this, I think that the only way is to choose the 20/40bit stuff in the menuconfig. > >> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile >> index 8303bcc..bab8c05 100644 >> --- a/drivers/scsi/ufs/Makefile >> +++ b/drivers/scsi/ufs/Makefile >> @@ -1,4 +1,7 @@ >> # UFSHCD makefile >> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o >> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o >> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o >> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o >> obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o >> obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o > > However, please split out the SCSI_UFS_DWC_TC
Re: [PATCH v9 3/3] add support for DWC UFS Host Controller
On Thursday 18 February 2016 17:20:27 Joao Pinto wrote:
>
> Documentation/devicetree/bindings/ufs/ufs-dwc.txt | 19 +
> MAINTAINERS | 6 +
> drivers/scsi/ufs/Kconfig | 51 +++
> drivers/scsi/ufs/Makefile | 3 +
> drivers/scsi/ufs/ufs-dwc-pci.c| 172 +
> drivers/scsi/ufs/ufs-dwc.c| 96 +
> drivers/scsi/ufs/ufshcd-dwc.c | 431
> ++
> drivers/scsi/ufs/ufshcd-dwc.h | 18 +
> drivers/scsi/ufs/ufshci-dwc.h | 42 +++
> drivers/scsi/ufs/unipro.h | 39 ++
I still think this can be split up further. From your previous
explanation, I understand that there is a specific test chip
that uses the "snps,ufshcd-dwc" implementation along with some
other glue logic.
Please split this out so you have anything related to the test
chips separate from the patch that implements core logic.
> diff --git a/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> new file mode 100644
> index 000..59e9822
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/ufs-dwc.txt
> @@ -0,0 +1,19 @@
> +* Universal Flash Storage (UFS) DesignWare Host Controller
> +
> +DWC_UFSHC nodes are defined to describe on-chip UFS host controllers.
> +Each UFS controller instance should have its own node.
> +
> +Required properties:
> +- compatible: compatible list must contain "snps,ufshcd-dwc" and
> should
> + also contain the JEDEC version of the controller:
> + "jedec,ufs-1.1"
> + "jedec,ufs-2.0"
> +- reg :
> +- interrupts:
Based on your last reply to me (sorry for coming back to this so
late), I think having just "snps,ufshcd-dwc" as the compatible
string is not appropriate: This assumes that all "snps,ufshcd-dwc"
instances are 100% compatible, but your code makes it very clear
that this is not the case.
Please for the last time (!) add a proper version number of the
specific IP block to the compatible strings so you can identify
what hardware you are talking to.
You earlier confused the version number of the UFS standard with
the version number of the controller, and that has been clarified
now, but you really still need to use a version for the hardware
as well.
> +config SCSI_UFS_DWC_TC
> + bool "Support for the Synopsys Test Chip"
> + depends on SCSI_UFS_DWC_HOOKS && (SCSI_UFSHCD_PCI || SCSI_UFS_DWC_PLAT)
> + ---help---
> + Synopsys Test Chip is a Phy for prototyping purposes.
> + This selects the support for the Synopsys Test Chip.
> +
> + Select this if you have a Synopsys Test Chip.
> + If unsure, say N.
> +
> +config SCSI_UFS_DWC_20BIT_RMMI
> + bool "20-bit RMMI support"
> + depends on SCSI_UFS_DWC_TC
> + ---help---
> + This specifies that the Synopsys Test Chip supports 20-bit RMMI.
> +
> + Select this if you are using a 20-bit RMMI Synopsys Test Chip.
> + If unsure, say N.
> +
> +config SCSI_UFS_DWC_40BIT_RMMI
> + bool "40-bit RMMI support"
> + depends on SCSI_UFS_DWC_TC
> + ---help---
> + Synopsys Test Chip is a Phy for prototyping purposes.
> +
> + Select this if you are using a 40-bit RMMI Synopsys Test Chip.
> + If unsure, say N.
I think it would be better to remove the SCSI_UFS_DWC_20BIT_RMMI
and SCSI_UFS_DWC_40BIT_RMMI configuration options now, and always
support both. There is not really much need for the options
as this is just a test chip, and nobody is going to care much
about saving a few bytes of object code.
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 8303bcc..bab8c05 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -1,4 +1,7 @@
> # UFSHCD makefile
> +obj-$(CONFIG_SCSI_UFS_DWC_HOOKS) += ufshcd-dwc.o
> +obj-$(CONFIG_SCSI_UFS_DWC_PLAT) += ufs-dwc.o
> +obj-$(CONFIG_SCSI_UFS_DWC_PCI) += ufs-dwc-pci.o
> obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o
> obj-$(CONFIG_SCSI_UFSHCD) += ufshcd.o
> obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
However, please split out the SCSI_UFS_DWC_TC specific bits into
a separate file, and put the module_platform_driver() bits into
that file, to get the right abstraction where the most specific
driver calls into the next driver, like
dwc-test-chip -> dwc-platform -> dwc -> ufs
It's possible you can leave out the dwc-platform level here, as there
are no other users for now, we can add others later as needed.
> +/**
> + * ufshcd_dwc_setup_tc()
> + * This function configures Local (host) Synopsys TC specific attributes
> + *
> + * @hba: Pointer to drivers structure
> + *
> + * Returns 0 on success non-zero value on failure
> + */
> +static int ufshcd_dwc_setup_tc(struct ufs_hba *hba)
> +{
> + int ret = 0;
> +
> +#ifdef
