diff mbox

[3/6] reset: hisilicon: add reset-hi3660

Message ID 1479800961-6249-4-git-send-email-zhangfei.gao@linaro.org
State New
Headers show

Commit Message

Zhangfei Gao Nov. 22, 2016, 7:49 a.m. UTC 
Add hi3660 reset driver based on common reset.c

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>

---
 drivers/reset/hisilicon/Kconfig                |  7 +++
 drivers/reset/hisilicon/Makefile               |  1 +
 drivers/reset/hisilicon/reset-hi3660.c         | 78 ++++++++++++++++++++++++++
 include/dt-bindings/reset/hisi,hi3660-resets.h | 38 +++++++++++++
 4 files changed, 124 insertions(+)
 create mode 100644 drivers/reset/hisilicon/reset-hi3660.c
 create mode 100644 include/dt-bindings/reset/hisi,hi3660-resets.h

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Arnd Bergmann Nov. 22, 2016, 8:50 a.m. UTC | #1 
On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:
> 

> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {

> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),

> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),

> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),

> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),

> +};

> +

> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {

> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),

> +       .channels = hi3660_iomcu_rst,

> +};

> +

> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {

> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),

> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),

> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),

> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),

> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),

> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),

> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),

> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),

> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),

> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),

> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),

> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),

> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),

> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),

> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),

> +};


I think you can avoid the trap of the ABI incompatibility if
you just define those as in the binding as tuples, using #reset-cells=2.

In particular for the first set, it seems really silly to redefine
the numbers when there is just a simple integer number.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhangfei Gao Nov. 22, 2016, 9:34 a.m. UTC | #2 
Hi, Arnd

On 2016年11月22日 16:50, Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:

>> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {

>> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),

>> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),

>> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),

>> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),

>> +};

>> +

>> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {

>> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),

>> +       .channels = hi3660_iomcu_rst,

>> +};

>> +

>> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {

>> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),

>> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),

>> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),

>> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),

>> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),

>> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),

>> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),

>> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),

>> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),

>> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),

>> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),

>> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),

>> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),

>> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),

>> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),

>> +};

> I think you can avoid the trap of the ABI incompatibility if

> you just define those as in the binding as tuples, using #reset-cells=2.

>

> In particular for the first set, it seems really silly to redefine

> the numbers when there is just a simple integer number.


Could you clarify more, still not understand.
The number is index of the arrays, and the index will be used in dts.
The arrays lists the registers offset and bit shift.
For example:

[HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.

And Documentation/devicetree/bindings/reset/reset.txt
Required properties:
#reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes
                 with a single reset output and 1 for nodes with multiple
                 reset outputs.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann Nov. 22, 2016, 9:42 a.m. UTC | #3 
On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:
> On 2016年11月22日 16:50, Arnd Bergmann wrote:

> > On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:

> >> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {

> >> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),

> >> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),

> >> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),

> >> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),

> >> +};

> >> +

> >> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {

> >> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),

> >> +       .channels = hi3660_iomcu_rst,

> >> +};

> >> +

> >> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {

> >> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),

> >> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),

> >> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),

> >> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),

> >> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),

> >> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),

> >> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),

> >> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),

> >> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),

> >> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),

> >> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),

> >> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),

> >> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),

> >> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),

> >> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),

> >> +};

> > I think you can avoid the trap of the ABI incompatibility if

> > you just define those as in the binding as tuples, using #reset-cells=2.

> >

> > In particular for the first set, it seems really silly to redefine

> > the numbers when there is just a simple integer number.

> 

> Could you clarify more, still not understand.

> The number is index of the arrays, and the index will be used in dts.

> The arrays lists the registers offset and bit shift.

> For example:

> 

> [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.

> 

> And Documentation/devicetree/bindings/reset/reset.txt

> Required properties:

> #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes

>                  with a single reset output and 1 for nodes with multiple

>                  reset outputs.


You can easily enumerate the registers that contain reset bits here,
so just use one cell for the register and another one for the index.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhangfei Gao Nov. 22, 2016, 10:02 a.m. UTC | #4 
On 2016年11月22日 17:42, Arnd Bergmann wrote:
> On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:

>> On 2016年11月22日 16:50, Arnd Bergmann wrote:

>>> On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:

>>>> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {

>>>> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),

>>>> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),

>>>> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),

>>>> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),

>>>> +};

>>>> +

>>>> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {

>>>> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),

>>>> +       .channels = hi3660_iomcu_rst,

>>>> +};

>>>> +

>>>> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {

>>>> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),

>>>> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),

>>>> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),

>>>> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),

>>>> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),

>>>> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),

>>>> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),

>>>> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),

>>>> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),

>>>> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),

>>>> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),

>>>> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),

>>>> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),

>>>> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),

>>>> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),

>>>> +};

>>> I think you can avoid the trap of the ABI incompatibility if

>>> you just define those as in the binding as tuples, using #reset-cells=2.

>>>

>>> In particular for the first set, it seems really silly to redefine

>>> the numbers when there is just a simple integer number.

>> Could you clarify more, still not understand.

>> The number is index of the arrays, and the index will be used in dts.

>> The arrays lists the registers offset and bit shift.

>> For example:

>>

>> [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.

>>

>> And Documentation/devicetree/bindings/reset/reset.txt

>> Required properties:

>> #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes

>>                   with a single reset output and 1 for nodes with multiple

>>                   reset outputs.

> You can easily enumerate the registers that contain reset bits here,

> so just use one cell for the register and another one for the index.


/* reset separated register offset is 0x4 */
#define HISI_RST_SEP(off, bit)                                  \
         { .enable       = REG_FIELD(off, bit, bit),             \
           .disable      = REG_FIELD(off + 0x4, bit, bit),       \
           .status       = REG_FIELD(off + 0x8, bit, bit), }

We not only provide the off and bit shift, but fulfill the members in 
the meantime.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Philipp Zabel Nov. 22, 2016, 10:22 a.m. UTC | #5 
Am Dienstag, den 22.11.2016, 10:42 +0100 schrieb Arnd Bergmann:
> On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:

> > On 2016年11月22日 16:50, Arnd Bergmann wrote:

> > > On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:

> > >> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {

> > >> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),

> > >> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),

> > >> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),

> > >> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),

> > >> +};

> > >> +

> > >> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {

> > >> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),

> > >> +       .channels = hi3660_iomcu_rst,

> > >> +};

> > >> +

> > >> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {

> > >> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),

> > >> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),

> > >> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),

> > >> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),

> > >> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),

> > >> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),

> > >> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),

> > >> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),

> > >> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),

> > >> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),

> > >> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),

> > >> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),

> > >> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),

> > >> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),

> > >> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),

> > >> +};

> > > I think you can avoid the trap of the ABI incompatibility if

> > > you just define those as in the binding as tuples, using #reset-cells=2.

> > >

> > > In particular for the first set, it seems really silly to redefine

> > > the numbers when there is just a simple integer number.

> > 

> > Could you clarify more, still not understand.

> > The number is index of the arrays, and the index will be used in dts.

> > The arrays lists the registers offset and bit shift.

> > For example:

> > 

> > [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.

> > 

> > And Documentation/devicetree/bindings/reset/reset.txt

> > Required properties:

> > #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes

> >                  with a single reset output and 1 for nodes with multiple

> >                  reset outputs.


This is just a suggestion, for reset controllers where the reset lines
can reasonably be enumerated by a single integer. If there is a good
reason to use more complicated bindings, more cells can be used.
That being said, I dislike having to spread register/bit information
throughout the device trees at the consumer/phandle sites, if the
register/bit information absolutely has to be put into the device tree,
I'd prefer a binding similar to ti-syscon, where it's all in one place.

> You can easily enumerate the registers that contain reset bits here,

> so just use one cell for the register and another one for the index.


Changing the reset cells is an incompatible change, and this is not a
straight forward register/bit mapping in hardware either. There are
currently three registers involved: enable (+0x0), disable (+0x4), and
status (+0x8). Also, what if in the future one of these reset bits have
to be handled inverted (as just happened for hi3519)?

regards
Philipp


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhangfei Gao Nov. 23, 2016, 8:02 a.m. UTC | #6 
Hi, Philipp


On 2016年11月22日 18:22, Philipp Zabel wrote:
> Am Dienstag, den 22.11.2016, 10:42 +0100 schrieb Arnd Bergmann:

>> On Tuesday, November 22, 2016 5:34:05 PM CET zhangfei wrote:

>>> On 2016年11月22日 16:50, Arnd Bergmann wrote:

>>>> On Tuesday, November 22, 2016 3:49:18 PM CET Zhangfei Gao wrote:

>>>>> +static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {

>>>>> +       [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),

>>>>> +       [HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),

>>>>> +       [HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),

>>>>> +       [HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),

>>>>> +};

>>>>> +

>>>>> +static struct hisi_reset_controller_data hi3660_iomcu_controller = {

>>>>> +       .nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),

>>>>> +       .channels = hi3660_iomcu_rst,

>>>>> +};

>>>>> +

>>>>> +static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {

>>>>> +       [HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),

>>>>> +       [HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),

>>>>> +       [HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),

>>>>> +       [HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),

>>>>> +       [HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),

>>>>> +       [HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),

>>>>> +       [HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),

>>>>> +       [HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),

>>>>> +       [HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),

>>>>> +       [HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),

>>>>> +       [HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),

>>>>> +       [HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),

>>>>> +       [HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),

>>>>> +       [HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),

>>>>> +       [HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),

>>>>> +};

>>>> I think you can avoid the trap of the ABI incompatibility if

>>>> you just define those as in the binding as tuples, using #reset-cells=2.

>>>>

>>>> In particular for the first set, it seems really silly to redefine

>>>> the numbers when there is just a simple integer number.

>>> Could you clarify more, still not understand.

>>> The number is index of the arrays, and the index will be used in dts.

>>> The arrays lists the registers offset and bit shift.

>>> For example:

>>>

>>> [HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3), means register offset : 0x20, and bit shift = 3.

>>>

>>> And Documentation/devicetree/bindings/reset/reset.txt

>>> Required properties:

>>> #reset-cells:   Number of cells in a reset specifier; Typically 0 for nodes

>>>                   with a single reset output and 1 for nodes with multiple

>>>                   reset outputs.

> This is just a suggestion, for reset controllers where the reset lines

> can reasonably be enumerated by a single integer. If there is a good

> reason to use more complicated bindings, more cells can be used.

> That being said, I dislike having to spread register/bit information

> throughout the device trees at the consumer/phandle sites, if the

> register/bit information absolutely has to be put into the device tree,

> I'd prefer a binding similar to ti-syscon, where it's all in one place.

Thanks for the suggestion.
Will use table in dts instead of 
include/dt-bindings/reset/hisi,hi3660-resets.h
like
+               hisi,reset-bits = <0x20 0x8             /* 0: i2c0 */
+                                  0x20 0x10            /* 1: i2c1 */
+                                  0x20 0x20            /* 2: i2c2 */
+                                  0x20 0x8000000>;     /* 3: i2c6 */

To remove the potential ABI issue as pointed by Arnd.
>

>> You can easily enumerate the registers that contain reset bits here,

>> so just use one cell for the register and another one for the index.

> Changing the reset cells is an incompatible change, and this is not a

> straight forward register/bit mapping in hardware either. There are

> currently three registers involved: enable (+0x0), disable (+0x4), and

> status (+0x8). Also, what if in the future one of these reset bits have

> to be handled inverted (as just happened for hi3519)?

Discussed with Jianchen, we are only considering Kirin series now.
The inverted in hi3519 is only for some line, not the whole controller.
It is more like a bug and kirin does not have such issue.

Will send a new RFC, help take a look.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/reset/hisilicon/Kconfig b/drivers/reset/hisilicon/Kconfig
index 1ff8b0c..10134dc 100644
--- a/drivers/reset/hisilicon/Kconfig
+++ b/drivers/reset/hisilicon/Kconfig
@@ -1,3 +1,10 @@ 
+config COMMON_RESET_HI3660
+	tristate "Hi3660 Reset Driver"
+	depends on ARCH_HISI || COMPILE_TEST
+	default ARCH_HISI
+	help
+	  Build the Hisilicon Hi3660 reset driver.
+
 config COMMON_RESET_HI6220
 	tristate "Hi6220 Reset Driver"
 	depends on ARCH_HISI || COMPILE_TEST
diff --git a/drivers/reset/hisilicon/Makefile b/drivers/reset/hisilicon/Makefile
index df511f5..57e9893 100644
--- a/drivers/reset/hisilicon/Makefile
+++ b/drivers/reset/hisilicon/Makefile
@@ -1,2 +1,3 @@ 
 obj-y	+= reset.o
+obj-$(CONFIG_COMMON_RESET_HI3660) += reset-hi3660.o
 obj-$(CONFIG_COMMON_RESET_HI6220) += hi6220_reset.o
diff --git a/drivers/reset/hisilicon/reset-hi3660.c b/drivers/reset/hisilicon/reset-hi3660.c
new file mode 100644
index 0000000..7da3153
--- /dev/null
+++ b/drivers/reset/hisilicon/reset-hi3660.c
@@ -0,0 +1,78 @@ 
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/reset/hisi,hi3660-resets.h>
+
+#include "reset.h"
+
+static const struct hisi_reset_channel_data hi3660_iomcu_rst[] = {
+	[HI3660_RST_I2C0] = HISI_RST_SEP(0x20, 3),
+	[HI3660_RST_I2C1] = HISI_RST_SEP(0x20, 4),
+	[HI3660_RST_I2C2] = HISI_RST_SEP(0x20, 5),
+	[HI3660_RST_I2C6] = HISI_RST_SEP(0x20, 27),
+};
+
+static struct hisi_reset_controller_data hi3660_iomcu_controller = {
+	.nr_channels = ARRAY_SIZE(hi3660_iomcu_rst),
+	.channels = hi3660_iomcu_rst,
+};
+
+static const struct hisi_reset_channel_data hi3660_crgctrl_rst[] = {
+	[HI3660_RST_I2C3] = HISI_RST_SEP(0x78, 7),
+	[HI3660_RST_I2C4] = HISI_RST_SEP(0x78, 27),
+	[HI3660_RST_I2C7] = HISI_RST_SEP(0x60, 14),
+	[HI3660_RST_SD] = HISI_RST_SEP(0x90, 18),
+	[HI3660_RST_SDIO] = HISI_RST_SEP(0x90, 20),
+	[HI3660_RST_UFS] = HISI_RST_SEP(0x84, 12),
+	[HI3660_RST_UFS_ASSERT] = HISI_RST_SEP(0x84, 7),
+	[HI3660_RST_PCIE_SYS] = HISI_RST_SEP(0x84, 26),
+	[HI3660_RST_PCIE_PHY] = HISI_RST_SEP(0x84, 27),
+	[HI3660_RST_PCIE_BUS] = HISI_RST_SEP(0x84, 31),
+	[HI3660_RST_USB3OTG_PHY] = HISI_RST_SEP(0x90, 3),
+	[HI3660_RST_USB3OTG] = HISI_RST_SEP(0x90, 5),
+	[HI3660_RST_USB3OTG_32K] = HISI_RST_SEP(0x90, 6),
+	[HI3660_RST_USB3OTG_AHB] = HISI_RST_SEP(0x90, 7),
+	[HI3660_RST_USB3OTG_MUX] = HISI_RST_SEP(0x90, 8),
+};
+
+static struct hisi_reset_controller_data hi3660_crgctrl_controller = {
+	.nr_channels = ARRAY_SIZE(hi3660_crgctrl_rst),
+	.channels = hi3660_crgctrl_rst,
+};
+
+static const struct of_device_id hi3660_reset_match[] = {
+	{ .compatible = "hisilicon,hi3660-reset-crgctrl",
+	  .data = &hi3660_crgctrl_controller, },
+	{ .compatible = "hisilicon,hi3660-reset-iomcu",
+	  .data = &hi3660_iomcu_controller, },
+	{},
+};
+MODULE_DEVICE_TABLE(of, hi3660_reset_match);
+
+static struct platform_driver hi3660_reset_driver = {
+	.probe = hisi_reset_probe,
+	.driver = {
+		.name = "reset-hi3660",
+		.of_match_table = hi3660_reset_match,
+	},
+};
+
+static int __init hi3660_reset_init(void)
+{
+	return platform_driver_register(&hi3660_reset_driver);
+}
+arch_initcall(hi3660_reset_init);
+
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:hi3660-reset");
+MODULE_DESCRIPTION("HiSilicon Hi3660 Reset Driver");
diff --git a/include/dt-bindings/reset/hisi,hi3660-resets.h b/include/dt-bindings/reset/hisi,hi3660-resets.h
new file mode 100644
index 0000000..a65f382
--- /dev/null
+++ b/include/dt-bindings/reset/hisi,hi3660-resets.h
@@ -0,0 +1,38 @@ 
+/*
+ * Copyright (c) 2016-2017 Linaro Ltd.
+ * Copyright (c) 2016-2017 HiSilicon Technologies Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef _DT_BINDINGS_RESET_CONTROLLER_HI3660
+#define _DT_BINDINGS_RESET_CONTROLLER_HI3660
+
+/* reset in iomcu */
+#define HI3660_RST_I2C0		0
+#define HI3660_RST_I2C1		1
+#define HI3660_RST_I2C2		2
+#define HI3660_RST_I2C6		3
+
+
+/* reset in crgctrl */
+#define HI3660_RST_I2C3		0
+#define HI3660_RST_I2C4		1
+#define HI3660_RST_I2C7		2
+#define HI3660_RST_SD		3
+#define HI3660_RST_SDIO		4
+#define HI3660_RST_UFS		5
+#define HI3660_RST_UFS_ASSERT	6
+#define HI3660_RST_PCIE_SYS	7
+#define HI3660_RST_PCIE_PHY	8
+#define HI3660_RST_PCIE_BUS	9
+#define HI3660_RST_USB3OTG_PHY  10
+#define HI3660_RST_USB3OTG	11
+#define HI3660_RST_USB3OTG_32K	12
+#define HI3660_RST_USB3OTG_AHB	13
+#define HI3660_RST_USB3OTG_MUX	14
+
+#endif /*_DT_BINDINGS_RESET_CONTROLLER_HI3660*/