| Message ID | 1445868903-183817-3-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Mon, Oct 26, 2015 at 10:14:33PM +0800, John Garry wrote: > Add devicetree bindings for HiSilicon SAS driver. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > .../devicetree/bindings/scsi/hisilicon-sas.txt | 70 ++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > > diff --git a/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > new file mode 100644 > index 0000000..d1e7b2a > --- /dev/null > +++ b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > @@ -0,0 +1,70 @@ > +* HiSilicon SAS controller > + > +The HiSilicon SAS controller supports SAS/SATA. > + > +Main node required properties: > + - compatible : value should be as follows: > + (a) "hisilicon,sas-controller-v1" for v1 of HiSilicon SAS controller IP > + - reg : Address and length of the SAS register > + - hisilicon,sas-syscon: phandle of syscon used for sas control > + - ctrl-reg : offset to the following SAS control registers (in order): > + - reset assert > + - clock disable > + - reset status > + - reset de-assert > + - clock enable This needs a better name, and it should probably be split up into several properties. However, it sounds like the syscon is actually a clock+reset controller, and should be modelled as such. It's not actually a part of the SAS controller as such. > + - queue-count : number of delivery and completion queues in the controller > + - phy-count : number of phys accessible by the controller > + - interrupts : Interrupts for phys, completion queues, and fatal > + interrupts: > + - Each phy has 3 interrupt sources: > + - broadcast > + - phyup > + - abnormal > + - Each completion queue has 1 interrupt source > + - Each controller has 2 fatal interrupt sources: > + - ECC > + - AXI bus Please make the ordering explicit here (you might only need to add the phrase "in order" in a few places). Thanks, Mark. > + > +* HiSilicon SAS syscon > + > +Required properties: > +- compatible: should be "hisilicon,sas-ctrl", "syscon" > +- reg: offset and length of the syscon sas-ctrl registers > + > + > +Example: > + sas_ctrl0: sas_ctrl@c0000000 { > + compatible = "hisilicon,sas-ctrl", "syscon"; > + reg = <0x0 0xc0000000 0x0 0x10000>; > + }; > + > + sas0: sas@c1000000 { > + compatible = "hisilicon,sas-controller-v1"; > + reg = <0x0 0xc1000000 0x0 0x10000>; > + hisilicon,sas-syscon = <&sas_ctrl0>; > + ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; > + queue-count = <32>; > + phy-count = <8>; > + dma-coherent; > + interrupt-parent = <&mbigen_dsa>; > + interrupts = <259 4>, <263 4>,<264 4>,/* phy irq(0~79) */ > + <269 4>,<273 4>,<274 4>,/* phy irq(0~79) */ > + <279 4>,<283 4>,<284 4>,/* phy irq(0~79) */ > + <289 4>,<293 4>,<294 4>,/* phy irq(0~79) */ > + <299 4>,<303 4>,<304 4>,/* phy irq(0~79) */ > + <309 4>,<313 4>,<314 4>,/* phy irq(0~79) */ > + <319 4>,<323 4>,<324 4>,/* phy irq(0~79) */ > + <329 4>,<333 4>,<334 4>,/* phy irq(0~79) */ > + <336 1>,<337 1>,<338 1>,<339 1>,<340 1>, > + <341 1>,<342 1>,<343 1>,/* cq irq (80~111) */ > + <344 1>,<345 1>,<346 1>,<347 1>,<348 1>, > + <349 1>,<350 1>,<351 1>,/* cq irq (80~111) */ > + <352 1>,<353 1>,<354 1>,<355 1>,<356 1>, > + <357 1>,<358 1>,<359 1>,/* cq irq (80~111) */ > + <360 1>,<361 1>,<362 1>,<363 1>,<364 1>, > + <365 1>,<366 1>,<367 1>,/* cq irq (80~111) */ > + <376 4>,/* chip fatal error irq(120) */ > + <381 4>;/* chip fatal error irq(125) */ > + status = "disabled"; > + }; > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/10/2015 14:14, John Garry wrote: > Add devicetree bindings for HiSilicon SAS driver. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > .../devicetree/bindings/scsi/hisilicon-sas.txt | 70 ++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > create mode 100644 Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > > diff --git a/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > new file mode 100644 > index 0000000..d1e7b2a > --- /dev/null > +++ b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > @@ -0,0 +1,70 @@ > +* HiSilicon SAS controller > + > +The HiSilicon SAS controller supports SAS/SATA. > + > +Main node required properties: > + - compatible : value should be as follows: > + (a) "hisilicon,sas-controller-v1" for v1 of HiSilicon SAS controller IP I accidently omitted new property sas-addr for v2 patchset; here is what it should look like: - sas-addr : array of 8 bytes for host SAS address > + - reg : Address and length of the SAS register > + - hisilicon,sas-syscon: phandle of syscon used for sas control > + - ctrl-reg : offset to the following SAS control registers (in order): > + - reset assert > + - clock disable > + - reset status > + - reset de-assert > + - clock enable > + - queue-count : number of delivery and completion queues in the controller > + - phy-count : number of phys accessible by the controller > + - interrupts : Interrupts for phys, completion queues, and fatal > + interrupts: > + - Each phy has 3 interrupt sources: > + - broadcast > + - phyup > + - abnormal > + - Each completion queue has 1 interrupt source > + - Each controller has 2 fatal interrupt sources: > + - ECC > + - AXI bus > + > +* HiSilicon SAS syscon > + > +Required properties: > +- compatible: should be "hisilicon,sas-ctrl", "syscon" > +- reg: offset and length of the syscon sas-ctrl registers > + > + > +Example: > + sas_ctrl0: sas_ctrl@c0000000 { > + compatible = "hisilicon,sas-ctrl", "syscon"; > + reg = <0x0 0xc0000000 0x0 0x10000>; > + }; > + > + sas0: sas@c1000000 { > + compatible = "hisilicon,sas-controller-v1"; sas-addr = [50 01 88 20 16 00 00 0a]; > + reg = <0x0 0xc1000000 0x0 0x10000>; > + hisilicon,sas-syscon = <&sas_ctrl0>; > + ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; > + queue-count = <32>; > + phy-count = <8>; > + dma-coherent; > + interrupt-parent = <&mbigen_dsa>; > + interrupts = <259 4>, <263 4>,<264 4>,/* phy irq(0~79) */ > + <269 4>,<273 4>,<274 4>,/* phy irq(0~79) */ > + <279 4>,<283 4>,<284 4>,/* phy irq(0~79) */ > + <289 4>,<293 4>,<294 4>,/* phy irq(0~79) */ > + <299 4>,<303 4>,<304 4>,/* phy irq(0~79) */ > + <309 4>,<313 4>,<314 4>,/* phy irq(0~79) */ > + <319 4>,<323 4>,<324 4>,/* phy irq(0~79) */ > + <329 4>,<333 4>,<334 4>,/* phy irq(0~79) */ > + <336 1>,<337 1>,<338 1>,<339 1>,<340 1>, > + <341 1>,<342 1>,<343 1>,/* cq irq (80~111) */ > + <344 1>,<345 1>,<346 1>,<347 1>,<348 1>, > + <349 1>,<350 1>,<351 1>,/* cq irq (80~111) */ > + <352 1>,<353 1>,<354 1>,<355 1>,<356 1>, > + <357 1>,<358 1>,<359 1>,/* cq irq (80~111) */ > + <360 1>,<361 1>,<362 1>,<363 1>,<364 1>, > + <365 1>,<366 1>,<367 1>,/* cq irq (80~111) */ > + <376 4>,/* chip fatal error irq(120) */ > + <381 4>;/* chip fatal error irq(125) */ > + status = "disabled"; > + }; > Comment on new property sas-addr added. John -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/10/2015 14:45, Mark Rutland wrote: > On Mon, Oct 26, 2015 at 10:14:33PM +0800, John Garry wrote: >> Add devicetree bindings for HiSilicon SAS driver. >> >> Signed-off-by: John Garry <john.garry@huawei.com> >> --- >> .../devicetree/bindings/scsi/hisilicon-sas.txt | 70 ++++++++++++++++++++++ >> 1 file changed, 70 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/scsi/hisilicon-sas.txt >> >> diff --git a/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt >> new file mode 100644 >> index 0000000..d1e7b2a >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt >> @@ -0,0 +1,70 @@ >> +* HiSilicon SAS controller >> + >> +The HiSilicon SAS controller supports SAS/SATA. >> + >> +Main node required properties: >> + - compatible : value should be as follows: >> + (a) "hisilicon,sas-controller-v1" for v1 of HiSilicon SAS controller IP >> + - reg : Address and length of the SAS register >> + - hisilicon,sas-syscon: phandle of syscon used for sas control >> + - ctrl-reg : offset to the following SAS control registers (in order): >> + - reset assert >> + - clock disable >> + - reset status >> + - reset de-assert >> + - clock enable > > This needs a better name, and it should probably be split up into > several properties. > > However, it sounds like the syscon is actually a clock+reset > controller, and should be modelled as such. It's not actually a part of > the SAS controller as such. The syscon block is a general subsystem control block, and it is not specifically only for controlling reset and enabling clocks (other functions include serdes control, for example). It is also shared with other peripherals. So we can remove the ctrl-reg property (since it is not part of the SAS controller), and add the relevant syscon register offsets to the "hisilicon,sas-syscon" property, like this: hisilicon,sas-syscon = <&sas_ctrl0 0xa60 0x33c 0x5a30 0xa64 0x338>; Ok? > >> + - queue-count : number of delivery and completion queues in the controller >> + - phy-count : number of phys accessible by the controller >> + - interrupts : Interrupts for phys, completion queues, and fatal >> + interrupts: >> + - Each phy has 3 interrupt sources: >> + - broadcast >> + - phyup >> + - abnormal >> + - Each completion queue has 1 interrupt source >> + - Each controller has 2 fatal interrupt sources: >> + - ECC >> + - AXI bus > > Please make the ordering explicit here (you might only need to add the > phrase "in order" in a few places). Will do. > > Thanks, > Mark. > Thanks, John -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 27, 2015 at 01:09:15PM +0000, John Garry wrote: > On 26/10/2015 14:45, Mark Rutland wrote: > >On Mon, Oct 26, 2015 at 10:14:33PM +0800, John Garry wrote: > >>Add devicetree bindings for HiSilicon SAS driver. > >> > >>Signed-off-by: John Garry <john.garry@huawei.com> > >>--- > >> .../devicetree/bindings/scsi/hisilicon-sas.txt | 70 ++++++++++++++++++++++ > >> 1 file changed, 70 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > >> > >>diff --git a/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > >>new file mode 100644 > >>index 0000000..d1e7b2a > >>--- /dev/null > >>+++ b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt > >>@@ -0,0 +1,70 @@ > >>+* HiSilicon SAS controller > >>+ > >>+The HiSilicon SAS controller supports SAS/SATA. > >>+ > >>+Main node required properties: > >>+ - compatible : value should be as follows: > >>+ (a) "hisilicon,sas-controller-v1" for v1 of HiSilicon SAS controller IP > >>+ - reg : Address and length of the SAS register > >>+ - hisilicon,sas-syscon: phandle of syscon used for sas control > >>+ - ctrl-reg : offset to the following SAS control registers (in order): > >>+ - reset assert > >>+ - clock disable > >>+ - reset status > >>+ - reset de-assert > >>+ - clock enable > > > >This needs a better name, and it should probably be split up into > >several properties. > > > >However, it sounds like the syscon is actually a clock+reset > >controller, and should be modelled as such. It's not actually a part of > >the SAS controller as such. > > The syscon block is a general subsystem control block, and it is not > specifically only for controlling reset and enabling clocks (other > functions include serdes control, for example). It is also shared > with other peripherals. > > So we can remove the ctrl-reg property (since it is not part of the > SAS controller), and add the relevant syscon register offsets to the > "hisilicon,sas-syscon" property, like this: > hisilicon,sas-syscon = <&sas_ctrl0 0xa60 0x33c 0x5a30 0xa64 0x338>; > > Ok? It would be better to have each offset in a separate property. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/27/2015 10:39 PM, Mark Rutland wrote: > On Tue, Oct 27, 2015 at 01:09:15PM +0000, John Garry wrote: >> On 26/10/2015 14:45, Mark Rutland wrote: >>> On Mon, Oct 26, 2015 at 10:14:33PM +0800, John Garry wrote: >>>> Add devicetree bindings for HiSilicon SAS driver. >>>> >>>> Signed-off-by: John Garry <john.garry@huawei.com> >>>> --- >>>> .../devicetree/bindings/scsi/hisilicon-sas.txt | 70 ++++++++++++++++++++++ >>>> 1 file changed, 70 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/scsi/hisilicon-sas.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt >>>> new file mode 100644 >>>> index 0000000..d1e7b2a >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt >>>> @@ -0,0 +1,70 @@ >>>> +* HiSilicon SAS controller >>>> + >>>> +The HiSilicon SAS controller supports SAS/SATA. >>>> + >>>> +Main node required properties: >>>> + - compatible : value should be as follows: >>>> + (a) "hisilicon,sas-controller-v1" for v1 of HiSilicon SAS controller IP >>>> + - reg : Address and length of the SAS register >>>> + - hisilicon,sas-syscon: phandle of syscon used for sas control >>>> + - ctrl-reg : offset to the following SAS control registers (in order): >>>> + - reset assert >>>> + - clock disable >>>> + - reset status >>>> + - reset de-assert >>>> + - clock enable >>> >>> This needs a better name, and it should probably be split up into >>> several properties. >>> >>> However, it sounds like the syscon is actually a clock+reset >>> controller, and should be modelled as such. It's not actually a part of >>> the SAS controller as such. >> >> The syscon block is a general subsystem control block, and it is not >> specifically only for controlling reset and enabling clocks (other >> functions include serdes control, for example). It is also shared >> with other peripherals. >> >> So we can remove the ctrl-reg property (since it is not part of the >> SAS controller), and add the relevant syscon register offsets to the >> "hisilicon,sas-syscon" property, like this: >> hisilicon,sas-syscon = <&sas_ctrl0 0xa60 0x33c 0x5a30 0xa64 0x338>; >> >> Ok? > > It would be better to have each offset in a separate property. > These register are not used for different purpose. Instead, they are all used for one purpose, reset the sas controller; Though a bit complicated, the silicon has special requirement here. So still prefer using the original method, ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; Since we can simply use of_property_read_u32_array. Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>>+ - ctrl-reg : offset to the following SAS control registers (in order): > >>>>+ - reset assert > >>>>+ - clock disable > >>>>+ - reset status > >>>>+ - reset de-assert > >>>>+ - clock enable [...] > >It would be better to have each offset in a separate property. > > > These register are not used for different purpose. > Instead, they are all used for one purpose, reset the sas controller; > Though a bit complicated, the silicon has special requirement here. While they are indeed used together, they are separate registers, as listed above. There may be other registers in future that turn out to be useful for the SAS controller, or it might turn out a new system doesn't have all of the existing registers. > So still prefer using the original method, > ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; > Since we can simply use of_property_read_u32_array. That may be simpler now, but makes future maintenance harder. Use separate properties. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> The syscon block is a general subsystem control block, and it is not >>> specifically only for controlling reset and enabling clocks (other >>> functions include serdes control, for example). It is also shared >>> with other peripherals. >>> >>> So we can remove the ctrl-reg property (since it is not part of the >>> SAS controller), and add the relevant syscon register offsets to the >>> "hisilicon,sas-syscon" property, like this: >>> hisilicon,sas-syscon = <&sas_ctrl0 0xa60 0x33c 0x5a30 0xa64 0x338>; >>> >>> Ok? >> >> It would be better to have each offset in a separate property. >> > These register are not used for different purpose. > Instead, they are all used for one purpose, reset the sas controller; > Though a bit complicated, the silicon has special requirement here. > > So still prefer using the original method, > ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; > Since we can simply use of_property_read_u32_array. > We can actually remove the deassert and clock enable properties as they are always at a fixed offset from their respective assert/clk disable partner register. > Thanks > > > . > thanks, John -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt new file mode 100644 index 0000000..d1e7b2a --- /dev/null +++ b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt @@ -0,0 +1,70 @@ +* HiSilicon SAS controller + +The HiSilicon SAS controller supports SAS/SATA. + +Main node required properties: + - compatible : value should be as follows: + (a) "hisilicon,sas-controller-v1" for v1 of HiSilicon SAS controller IP + - reg : Address and length of the SAS register + - hisilicon,sas-syscon: phandle of syscon used for sas control + - ctrl-reg : offset to the following SAS control registers (in order): + - reset assert + - clock disable + - reset status + - reset de-assert + - clock enable + - queue-count : number of delivery and completion queues in the controller + - phy-count : number of phys accessible by the controller + - interrupts : Interrupts for phys, completion queues, and fatal + interrupts: + - Each phy has 3 interrupt sources: + - broadcast + - phyup + - abnormal + - Each completion queue has 1 interrupt source + - Each controller has 2 fatal interrupt sources: + - ECC + - AXI bus + +* HiSilicon SAS syscon + +Required properties: +- compatible: should be "hisilicon,sas-ctrl", "syscon" +- reg: offset and length of the syscon sas-ctrl registers + + +Example: + sas_ctrl0: sas_ctrl@c0000000 { + compatible = "hisilicon,sas-ctrl", "syscon"; + reg = <0x0 0xc0000000 0x0 0x10000>; + }; + + sas0: sas@c1000000 { + compatible = "hisilicon,sas-controller-v1"; + reg = <0x0 0xc1000000 0x0 0x10000>; + hisilicon,sas-syscon = <&sas_ctrl0>; + ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; + queue-count = <32>; + phy-count = <8>; + dma-coherent; + interrupt-parent = <&mbigen_dsa>; + interrupts = <259 4>, <263 4>,<264 4>,/* phy irq(0~79) */ + <269 4>,<273 4>,<274 4>,/* phy irq(0~79) */ + <279 4>,<283 4>,<284 4>,/* phy irq(0~79) */ + <289 4>,<293 4>,<294 4>,/* phy irq(0~79) */ + <299 4>,<303 4>,<304 4>,/* phy irq(0~79) */ + <309 4>,<313 4>,<314 4>,/* phy irq(0~79) */ + <319 4>,<323 4>,<324 4>,/* phy irq(0~79) */ + <329 4>,<333 4>,<334 4>,/* phy irq(0~79) */ + <336 1>,<337 1>,<338 1>,<339 1>,<340 1>, + <341 1>,<342 1>,<343 1>,/* cq irq (80~111) */ + <344 1>,<345 1>,<346 1>,<347 1>,<348 1>, + <349 1>,<350 1>,<351 1>,/* cq irq (80~111) */ + <352 1>,<353 1>,<354 1>,<355 1>,<356 1>, + <357 1>,<358 1>,<359 1>,/* cq irq (80~111) */ + <360 1>,<361 1>,<362 1>,<363 1>,<364 1>, + <365 1>,<366 1>,<367 1>,/* cq irq (80~111) */ + <376 4>,/* chip fatal error irq(120) */ + <381 4>;/* chip fatal error irq(125) */ + status = "disabled"; + };
Add devicetree bindings for HiSilicon SAS driver. Signed-off-by: John Garry <john.garry@huawei.com> --- .../devicetree/bindings/scsi/hisilicon-sas.txt | 70 ++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/scsi/hisilicon-sas.txt