Re: [linux-sunxi] Re: [PATCH 2/2] ASoC: sunxi: compatibility for sun6i to SPDIF

[Maxime Ripard]

Hi,

On Mon, Aug 01, 2016 at 09:39:34PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Sat, Jul 30, 2016 at 11:20 PM, maxime.rip...@free-electrons.com
>  wrote:
> > On Sat, Jul 30, 2016 at 10:52:45PM +0800, Icenowy Zheng wrote:
> >> > + if (of_device_is_compatible(pdev->dev.of_node,
> >> > + "allwinner,sun6i-a31-spdif")) {
> >> > + host->rst = devm_reset_control_get_optional(>dev, NULL);
> >> > + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
> >> > + ret = -EPROBE_DEFER;
> >> > + dev_err(>dev, "Failed to get reset: %d\n", ret);
> >> > + goto err_disable_apb_clk;
> >> > + }
> >> > + if (!IS_ERR(host->rst))
> >> > + reset_control_deassert(host->rst);
> >> > + }
> >> > +
> >> I think you do not need the compatible.
> >> You can just detect whether the reset is present.
> >
> > That would weaken the error check. If we're running on the A31 and are
> > missing our reset property, it would go unnoticed.
> 
> We've been doing it this way with the mmc controller and the usb hosts though.
> IIRC you once said in the older SoCs, the reset control is tied to the clock
> gate in the hardware.
> 
> The _optional variant is also funny, though I understand it is a design
> of the reset controller framework.

Yes, I know. But that doesn't prevent that design from being better.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [linux-sunxi] Re: [PATCH 2/2] ASoC: sunxi: compatibility for sun6i to SPDIF

[Maxime Ripard]

Hi,

On Mon, Aug 01, 2016 at 09:39:34PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Sat, Jul 30, 2016 at 11:20 PM, maxime.rip...@free-electrons.com
>  wrote:
> > On Sat, Jul 30, 2016 at 10:52:45PM +0800, Icenowy Zheng wrote:
> >> > + if (of_device_is_compatible(pdev->dev.of_node,
> >> > + "allwinner,sun6i-a31-spdif")) {
> >> > + host->rst = devm_reset_control_get_optional(>dev, NULL);
> >> > + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
> >> > + ret = -EPROBE_DEFER;
> >> > + dev_err(>dev, "Failed to get reset: %d\n", ret);
> >> > + goto err_disable_apb_clk;
> >> > + }
> >> > + if (!IS_ERR(host->rst))
> >> > + reset_control_deassert(host->rst);
> >> > + }
> >> > +
> >> I think you do not need the compatible.
> >> You can just detect whether the reset is present.
> >
> > That would weaken the error check. If we're running on the A31 and are
> > missing our reset property, it would go unnoticed.
> 
> We've been doing it this way with the mmc controller and the usb hosts though.
> IIRC you once said in the older SoCs, the reset control is tied to the clock
> gate in the hardware.
> 
> The _optional variant is also funny, though I understand it is a design
> of the reset controller framework.

Yes, I know. But that doesn't prevent that design from being better.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [linux-sunxi] Re: [PATCH 2/2] ASoC: sunxi: compatibility for sun6i to SPDIF

[Chen-Yu Tsai]

Hi,

On Sat, Jul 30, 2016 at 11:20 PM, maxime.rip...@free-electrons.com
 wrote:
> On Sat, Jul 30, 2016 at 10:52:45PM +0800, Icenowy Zheng wrote:
>> > + if (of_device_is_compatible(pdev->dev.of_node,
>> > + "allwinner,sun6i-a31-spdif")) {
>> > + host->rst = devm_reset_control_get_optional(>dev, NULL);
>> > + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
>> > + ret = -EPROBE_DEFER;
>> > + dev_err(>dev, "Failed to get reset: %d\n", ret);
>> > + goto err_disable_apb_clk;
>> > + }
>> > + if (!IS_ERR(host->rst))
>> > + reset_control_deassert(host->rst);
>> > + }
>> > +
>> I think you do not need the compatible.
>> You can just detect whether the reset is present.
>
> That would weaken the error check. If we're running on the A31 and are
> missing our reset property, it would go unnoticed.

We've been doing it this way with the mmc controller and the usb hosts though.
IIRC you once said in the older SoCs, the reset control is tied to the clock
gate in the hardware.

The _optional variant is also funny, though I understand it is a design
of the reset controller framework.


Regards
ChenYu

Re: [linux-sunxi] Re: [PATCH 2/2] ASoC: sunxi: compatibility for sun6i to SPDIF

[Chen-Yu Tsai]

Hi,

On Sat, Jul 30, 2016 at 11:20 PM, maxime.rip...@free-electrons.com
 wrote:
> On Sat, Jul 30, 2016 at 10:52:45PM +0800, Icenowy Zheng wrote:
>> > + if (of_device_is_compatible(pdev->dev.of_node,
>> > + "allwinner,sun6i-a31-spdif")) {
>> > + host->rst = devm_reset_control_get_optional(>dev, NULL);
>> > + if (IS_ERR(host->rst) && PTR_ERR(host->rst) == -EPROBE_DEFER) {
>> > + ret = -EPROBE_DEFER;
>> > + dev_err(>dev, "Failed to get reset: %d\n", ret);
>> > + goto err_disable_apb_clk;
>> > + }
>> > + if (!IS_ERR(host->rst))
>> > + reset_control_deassert(host->rst);
>> > + }
>> > +
>> I think you do not need the compatible.
>> You can just detect whether the reset is present.
>
> That would weaken the error check. If we're running on the A31 and are
> missing our reset property, it would go unnoticed.

We've been doing it this way with the mmc controller and the usb hosts though.
IIRC you once said in the older SoCs, the reset control is tied to the clock
gate in the hardware.

The _optional variant is also funny, though I understand it is a design
of the reset controller framework.


Regards
ChenYu