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@xxxxxxxxxx> > >>--- > >> .../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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html