| Message ID | 1384457519-21335-2-git-send-email-lho@apm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday 14 November 2013, Loc Ho wrote: > +- id : PHY ID (0 = first, 1 = second, 2 = third) > +- #phy-cells : Shall be 0 Hmm, it seems like this would make more sense if you have #phy-cells=<1> and have the PHY ID in user node as the argument. > +- CTLE0 : PHY override parameters for channel 0 register REG1 > + field CTLE_EQ. First value for Gen1, second value > + for Gen2, and third value for Gen3. Default is 0x2. > +- CTLE1 : PHY override parameters for channel 1 register REG1 > + field CTLE_EQ. First value for Gen1, second value > + for Gen2, and third value for Gen3. Default is 0x2. > +- PQ0 : PHY override parameters for channel 0 register REG125 > + field PQ_REG. First value for Gen1, second value > + for Gen2, and third value for Gen3. Default is 0xA. > +- PQ1 : PHY override parameters for channel 1 register REG125 > + field PQ_REG. First value for Gen1, second value > + for Gen2, and third value for Gen3. Default is 0xA. As mentioned before, I don't think putting register-level information into the binding is the right approach here. Arnd
On Friday 15 November 2013, Arnd Bergmann wrote: > On Thursday 14 November 2013, Loc Ho wrote: > > +- id : PHY ID (0 = first, 1 = second, 2 = third) > > +- #phy-cells : Shall be 0 > > Hmm, it seems like this would make more sense if you have #phy-cells=<1> > and have the PHY ID in user node as the argument. Ignore this comment. After reading patch 2, I concluded that your choice of #phy-cells=0 is probably more appropriate here. Arnd
Hi, >> +- CTLE0 : PHY override parameters for channel 0 register REG1 >> + field CTLE_EQ. First value for Gen1, second value >> + for Gen2, and third value for Gen3. Default is 0x2. >> +- CTLE1 : PHY override parameters for channel 1 register REG1 >> + field CTLE_EQ. First value for Gen1, second value >> + for Gen2, and third value for Gen3. Default is 0x2. >> +- PQ0 : PHY override parameters for channel 0 register REG125 >> + field PQ_REG. First value for Gen1, second value >> + for Gen2, and third value for Gen3. Default is 0xA. >> +- PQ1 : PHY override parameters for channel 1 register REG125 >> + field PQ_REG. First value for Gen1, second value >> + for Gen2, and third value for Gen3. Default is 0xA. > > As mentioned before, I don't think putting register-level information into the binding > is the right approach here. > [Loc Ho] I don't want to change the driver for every possible customer or APM boards out there. In general, if the board designer follows the board design guideline, this isn't necessary. Unfortunately, I can not control what customer do. If these setting is NOT driven by DTS, then I or others may have to change the driver every time there is an new board. Another option is pass them as module parameters. -Loc
On Friday 15 November 2013, Loc Ho wrote: > >> +- CTLE0 : PHY override parameters for channel 0 register REG1 > >> + field CTLE_EQ. First value for Gen1, second value > >> + for Gen2, and third value for Gen3. Default is 0x2. > >> +- CTLE1 : PHY override parameters for channel 1 register REG1 > >> + field CTLE_EQ. First value for Gen1, second value > >> + for Gen2, and third value for Gen3. Default is 0x2. > >> +- PQ0 : PHY override parameters for channel 0 register REG125 > >> + field PQ_REG. First value for Gen1, second value > >> + for Gen2, and third value for Gen3. Default is 0xA. > >> +- PQ1 : PHY override parameters for channel 1 register REG125 > >> + field PQ_REG. First value for Gen1, second value > >> + for Gen2, and third value for Gen3. Default is 0xA. > > > > As mentioned before, I don't think putting register-level information into the binding > > is the right approach here. > > > [Loc Ho] > I don't want to change the driver for every possible customer or APM > boards out there. In general, if the board designer follows the board > design guideline, this isn't necessary. Unfortunately, I can not > control what customer do. If these setting is NOT driven by DTS, then > I or others may have to change the driver every time there is an new > board. Another option is pass them as module parameters. Module parameters are certainly not a solution for this, that would make the problem worse. Would it be possible to read out the values that were set by the firmware at startup and reuse those during run-time? I assume that the devices are already brought up when the kernel starts. If that s not possible, change the properties to be named after what the setting does and make the values be units that are sensible to a board designer, e.g. (specifics are totally made up here) - polarity: If present, override the polarity of the signal line. zero for negative polarity, one for positive polarity. If absent, use the firmware default. - signal-clock-frequency: If present, override the signal clock frequency. This value is defined in HZ - x-y-time: If present, override the timing setting for the X/Y signal time, in nanoseconds. This way, you can reuse the properties for an updated version of the device that has similar settings but a different register layout. Arnd
Hi, On Fri, Nov 15, 2013 at 11:26 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Friday 15 November 2013, Loc Ho wrote: >> >> +- CTLE0 : PHY override parameters for channel 0 register REG1 >> >> + field CTLE_EQ. First value for Gen1, second value >> >> + for Gen2, and third value for Gen3. Default is 0x2. >> >> +- CTLE1 : PHY override parameters for channel 1 register REG1 >> >> + field CTLE_EQ. First value for Gen1, second value >> >> + for Gen2, and third value for Gen3. Default is 0x2. >> >> +- PQ0 : PHY override parameters for channel 0 register REG125 >> >> + field PQ_REG. First value for Gen1, second value >> >> + for Gen2, and third value for Gen3. Default is 0xA. >> >> +- PQ1 : PHY override parameters for channel 1 register REG125 >> >> + field PQ_REG. First value for Gen1, second value >> >> + for Gen2, and third value for Gen3. Default is 0xA. >> > >> > As mentioned before, I don't think putting register-level information into the binding >> > is the right approach here. >> > >> [Loc Ho] >> I don't want to change the driver for every possible customer or APM >> boards out there. In general, if the board designer follows the board >> design guideline, this isn't necessary. Unfortunately, I can not >> control what customer do. If these setting is NOT driven by DTS, then >> I or others may have to change the driver every time there is an new >> board. Another option is pass them as module parameters. > > Module parameters are certainly not a solution for this, that would make the > problem worse. > > Would it be possible to read out the values that were set by the firmware > at startup and reuse those during run-time? I assume that the devices are > already brought up when the kernel starts. > > If that s not possible, change the properties to be named after what the > setting does and make the values be units that are sensible to a board > designer, e.g. (specifics are totally made up here) > > - polarity: If present, override the polarity of the signal line. > zero for negative polarity, one for positive polarity. > If absent, use the firmware default. > - signal-clock-frequency: If present, override the signal clock frequency. > This value is defined in HZ > - x-y-time: If present, override the timing setting for the X/Y signal > time, in nanoseconds. > > This way, you can reuse the properties for an updated version of the device > that has similar settings but a different register layout. > [Loc Ho] It may be possible from ACPI point of view (hasn't tried yet). But for DTS, there is no such firmware. I will make the entry more friendly for DTS. For ACPI, I am thinking that the BIOS/Firmware will just initialize it. As long as the clock isn't reset, the value will be there. Let's agree on these: 1. DTS - use more friendly name 2. ACPI - handled by the BIOS/Firmware -Loc
On Friday 15 November 2013, Loc Ho wrote: > It may be possible from ACPI point of view (hasn't tried yet). But for > DTS, there is no such firmware. Why not? Doesn't any boot loader have to initialize the hardware in order to load a kernel from disk? > I will make the entry more friendly > for DTS. For ACPI, I am thinking that the BIOS/Firmware will just > initialize it. As long as the clock isn't reset, the value will be > there. Let's agree on these: > > 1. DTS - use more friendly name > 2. ACPI - handled by the BIOS/Firmware Make sense. Arnd
Hi, >> It may be possible from ACPI point of view (hasn't tried yet). But for >> DTS, there is no such firmware. > > Why not? Doesn't any boot loader have to initialize the hardware in order > to load a kernel from disk? [Loc Ho] Not necessary for network booting, SD boot, and USB boot with ramdisk. -Loc
On Friday 15 November 2013, Loc Ho wrote: > Hi, > > >> It may be possible from ACPI point of view (hasn't tried yet). But for > >> DTS, there is no such firmware. > > > > Why not? Doesn't any boot loader have to initialize the hardware in order > > to load a kernel from disk? > [Loc Ho] > Not necessary for network booting, SD boot, and USB boot with ramdisk. But why not just initialize them all if you already have the boot loader contain all the drivers? There really isn't any difference between passing an ACPI blob or a DT blob to the kernel, as far as the boot loader is concerned. Arnd
Hi, >> >> It may be possible from ACPI point of view (hasn't tried yet). But for >> >> DTS, there is no such firmware. >> > >> > Why not? Doesn't any boot loader have to initialize the hardware in order >> > to load a kernel from disk? >> [Loc Ho] >> Not necessary for network booting, SD boot, and USB boot with ramdisk. > > But why not just initialize them all if you already have the boot loader > contain all the drivers? There really isn't any difference between > passing an ACPI blob or a DT blob to the kernel, as far as the boot loader > is concerned. > [Loc Ho] Yes... It is a possible solution. The draw back would be slow down the boot loader. For now, let me implement the PHY for DTS and no PHY for ACPI. From previous emails, it was discussed that PHY driver is required for DTS. For ACPI, it is the responsible of the firmware. -Loc
diff --git a/Documentation/devicetree/bindings/ata/apm-xgene.txt b/Documentation/devicetree/bindings/ata/apm-xgene.txt new file mode 100644 index 0000000..d18db67 --- /dev/null +++ b/Documentation/devicetree/bindings/ata/apm-xgene.txt @@ -0,0 +1,69 @@ +* APM X-Gene 6.0 Gb/s SATA PHY nodes + +SATA PHY nodes are defined to describe on-chip Serial ATA PHY. Each SATA PHY +(pair of PHY) has its own node. + +Required properties: +- compatible : Shall be "apm,xgene-ahci-phy" +- reg : First memory resource shall be the PHY memory resource + Second memory resource shall be the optional PHY + memory resource if mux'ed with another IP +- id : PHY ID (0 = first, 1 = second, 2 = third) +- #phy-cells : Shall be 0 + +Optional properties: +- status : Shall be "ok" if enabled or "na" if disabled. Default + is "ok". +- CTLE0 : PHY override parameters for channel 0 register REG1 + field CTLE_EQ. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0x2. +- CTLE1 : PHY override parameters for channel 1 register REG1 + field CTLE_EQ. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0x2. +- PQ0 : PHY override parameters for channel 0 register REG125 + field PQ_REG. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0xA. +- PQ1 : PHY override parameters for channel 1 register REG125 + field PQ_REG. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0xA. +- PQS0 : PHY override parameters for channel 0 register REG125 + field PQ_SIGN. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0x1. +- PQS1 : PHY override parameters for channel 1 register REG125 + field PQ_SIGN. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0x1. +- SPD0 : PHY override parameters for channel 0 register REG61 + field PQ_SIGN. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0x5. +- SPD1 : PHY override parameters for channel 1 register REG61 + field PQ_SIGN. First value for Gen1, second value + for Gen2, and third value for Gen3. Default is 0x5. + +NOTE: PHY override parameters are board specific setting. + +Example: + sataphy0: sataphy@1f210000 { + compatible = "apm,xgene-ahci-phy"; + id = <0>; + reg = <0x0 0x1f210000 0x0 0x10000>; + #phy-cells = <0>; + status = "na"; + }; + + sataphy1: sataphy@1f220000 { + compatible = "apm,xgene-ahci-phy"; + id = <1>; + reg = <0x0 0x1f220000 0x0 0x10000>; + #phy-cells = <0>; + status = "ok"; + }; + + sataphy2: sataphy@1f230000 { + compatible = "apm,xgene-ahci-phy"; + id = <2>; + reg = <0x0 0x1f230000 0x0 0x10000 + 0x0 0x1f2d0000 0x0 0x10000 >; + #phy-cells = <0>; + status = "ok"; + }; +