| Message ID | 1418836704-15689-5-git-send-email-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede@redhat.com> wrote: > On sun6i the cir block is attached to the reset controller, add support > for de-asserting the reset if a reset controller is specified in dt. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> > --- > .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ > drivers/media/rc/sunxi-cir.c | 25 ++++++++++++++++++++-- > 2 files changed, 25 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt > index 23dd5ad..6b70b9b 100644 > --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt > +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt > @@ -10,6 +10,7 @@ Required properties: > > Optional properties: > - linux,rc-map-name : Remote control map name. > +- resets : phandle + reset specifier pair Should it be optional? Or should we use a sun6i compatible with a mandatory reset phandle? I mean, the driver/hardware is not going to work with the reset missing on sun6i. Seems we are doing it one way for some of our drivers, and the other (optional) way for more generic ones, like USB. ChenYu > Example: > > @@ -17,6 +18,7 @@ ir0: ir@01c21800 { > compatible = "allwinner,sun4i-a10-ir"; > clocks = <&apb0_gates 6>, <&ir0_clk>; > clock-names = "apb", "ir"; > + resets = <&apb0_rst 1>; > interrupts = <0 5 1>; > reg = <0x01C21800 0x40>; > linux,rc-map-name = "rc-rc6-mce"; > diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c > index 340f7f5..06170e0 100644 > --- a/drivers/media/rc/sunxi-cir.c > +++ b/drivers/media/rc/sunxi-cir.c > @@ -23,6 +23,7 @@ > #include <linux/interrupt.h> > #include <linux/module.h> > #include <linux/of_platform.h> > +#include <linux/reset.h> > #include <media/rc-core.h> > > #define SUNXI_IR_DEV "sunxi-ir" > @@ -95,6 +96,7 @@ struct sunxi_ir { > int irq; > struct clk *clk; > struct clk *apb_clk; > + struct reset_control *rst; > const char *map_name; > }; > > @@ -166,15 +168,29 @@ static int sunxi_ir_probe(struct platform_device *pdev) > return PTR_ERR(ir->clk); > } > > + /* Reset (optional) */ > + ir->rst = devm_reset_control_get_optional(dev, NULL); > + if (IS_ERR(ir->rst)) { > + ret = PTR_ERR(ir->rst); > + if (ret == -EPROBE_DEFER) > + return ret; > + ir->rst = NULL; > + } else { > + ret = reset_control_deassert(ir->rst); > + if (ret) > + return ret; > + } > + > ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK); > if (ret) { > dev_err(dev, "set ir base clock failed!\n"); > - return ret; > + goto exit_reset_assert; > } > > if (clk_prepare_enable(ir->apb_clk)) { > dev_err(dev, "try to enable apb_ir_clk failed\n"); > - return -EINVAL; > + ret = -EINVAL; > + goto exit_reset_assert; > } > > if (clk_prepare_enable(ir->clk)) { > @@ -271,6 +287,9 @@ exit_clkdisable_clk: > clk_disable_unprepare(ir->clk); > exit_clkdisable_apb_clk: > clk_disable_unprepare(ir->apb_clk); > +exit_reset_assert: > + if (ir->rst) > + reset_control_assert(ir->rst); > > return ret; > } > @@ -282,6 +301,8 @@ static int sunxi_ir_remove(struct platform_device *pdev) > > clk_disable_unprepare(ir->clk); > clk_disable_unprepare(ir->apb_clk); > + if (ir->rst) > + reset_control_assert(ir->rst); > > spin_lock_irqsave(&ir->ir_lock, flags); > /* disable IR IRQ */ > -- > 2.1.0 > > -- > You received this message because you are subscribed to the Google Groups "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 18-12-14 03:48, Chen-Yu Tsai wrote: > Hi, > > On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede@redhat.com> wrote: >> On sun6i the cir block is attached to the reset controller, add support >> for de-asserting the reset if a reset controller is specified in dt. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> >> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> >> --- >> .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ >> drivers/media/rc/sunxi-cir.c | 25 ++++++++++++++++++++-- >> 2 files changed, 25 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt >> index 23dd5ad..6b70b9b 100644 >> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt >> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt >> @@ -10,6 +10,7 @@ Required properties: >> >> Optional properties: >> - linux,rc-map-name : Remote control map name. >> +- resets : phandle + reset specifier pair > > Should it be optional? Or should we use a sun6i compatible with > a mandatory reset phandle? I mean, the driver/hardware is not > going to work with the reset missing on sun6i. > > Seems we are doing it one way for some of our drivers, and > the other (optional) way for more generic ones, like USB. I do not believe that we should add a new compatible just because the reset line of a block is hooked up differently. It is the exact same ip-block. Only now the reset is not controlled through the apb-gate, but controlled separately. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote: > Hi, > > On 18-12-14 03:48, Chen-Yu Tsai wrote: > >Hi, > > > >On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede@redhat.com> wrote: > >>On sun6i the cir block is attached to the reset controller, add support > >>for de-asserting the reset if a reset controller is specified in dt. > >> > >>Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >>Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > >>Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> > >>--- > >> .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ > >> drivers/media/rc/sunxi-cir.c | 25 ++++++++++++++++++++-- > >> 2 files changed, 25 insertions(+), 2 deletions(-) > >> > >>diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt > >>index 23dd5ad..6b70b9b 100644 > >>--- a/Documentation/devicetree/bindings/media/sunxi-ir.txt > >>+++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt > >>@@ -10,6 +10,7 @@ Required properties: > >> > >> Optional properties: > >> - linux,rc-map-name : Remote control map name. > >>+- resets : phandle + reset specifier pair > > > >Should it be optional? Or should we use a sun6i compatible with > >a mandatory reset phandle? I mean, the driver/hardware is not > >going to work with the reset missing on sun6i. > > > >Seems we are doing it one way for some of our drivers, and > >the other (optional) way for more generic ones, like USB. > > I do not believe that we should add a new compatible just because > the reset line of a block is hooked up differently. It is the > exact same ip-block. Only now the reset is not controlled > through the apb-gate, but controlled separately. He has a point though. Your driver might very well probe nicely and everything, but still wouldn't be functional at all because the reset line wouldn't have been specified in the DT. The easiest way to deal with that would be in the bindings doc to update it with a compatible for the A31, and mentionning that the reset property is mandatory there. Note that the code itself might not change at all though. I'd just like to avoid any potential breaking of the DT bindings themselves. If we further want to refine the code, we can do that however we want. I have a slight preference for a clean error if reset is missing, but I won't get in the way just for that. Maxime
Hi, On 19-12-14 19:17, Maxime Ripard wrote: > Hi, > > On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote: >> Hi, >> >> On 18-12-14 03:48, Chen-Yu Tsai wrote: >>> Hi, >>> >>> On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede@redhat.com> wrote: >>>> On sun6i the cir block is attached to the reset controller, add support >>>> for de-asserting the reset if a reset controller is specified in dt. >>>> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> >>>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>> --- >>>> .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ >>>> drivers/media/rc/sunxi-cir.c | 25 ++++++++++++++++++++-- >>>> 2 files changed, 25 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>> index 23dd5ad..6b70b9b 100644 >>>> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>> @@ -10,6 +10,7 @@ Required properties: >>>> >>>> Optional properties: >>>> - linux,rc-map-name : Remote control map name. >>>> +- resets : phandle + reset specifier pair >>> >>> Should it be optional? Or should we use a sun6i compatible with >>> a mandatory reset phandle? I mean, the driver/hardware is not >>> going to work with the reset missing on sun6i. >>> >>> Seems we are doing it one way for some of our drivers, and >>> the other (optional) way for more generic ones, like USB. >> >> I do not believe that we should add a new compatible just because >> the reset line of a block is hooked up differently. It is the >> exact same ip-block. Only now the reset is not controlled >> through the apb-gate, but controlled separately. > > He has a point though. Your driver might very well probe nicely and > everything, but still wouldn't be functional at all because the reset > line wouldn't have been specified in the DT. Right, just like other drivers we've, see e.g.: Documentation/devicetree/bindings/mmc/sunxi-mmc.txt Which is dealing with this in the same way. > The easiest way to deal with that would be in the bindings doc to > update it with a compatible for the A31, and mentionning that the > reset property is mandatory there. No the easiest way to deal with this is to expect people writing the dts to know what they are doing, just like we do for a lot of the other blocks in sun6i. Maybe put a generic note somewhere that sun6i has a reset controller, and that for all the blocks with optional resets property it should be considered mandatory on sun6i ? I'm sorry but I'm not going to make this change for the ir bindings given that we've the same situation in a lot of other places. Consistency is important. Moreover I believe that having a sun6i specific compatible string is just wrong, since it is the exact same hardware block as on sun5i, just with its reset line routed differently, just like e.g. the mmc controller, the uarts or the gmac all of which also do not have a sun6i specific compatible to enforce reset controller usage. Regards, Hans > Note that the code itself might not change at all though. I'd just > like to avoid any potential breaking of the DT bindings themselves. If > we further want to refine the code, we can do that however we want. > > I have a slight preference for a clean error if reset is missing, but > I won't get in the way just for that. > > Maxime > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Sat, Dec 20, 2014 at 6:20 PM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > > On 19-12-14 19:17, Maxime Ripard wrote: >> >> Hi, >> >> On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote: >>> >>> Hi, >>> >>> On 18-12-14 03:48, Chen-Yu Tsai wrote: >>>> >>>> Hi, >>>> >>>> On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede@redhat.com> >>>> wrote: >>>>> >>>>> On sun6i the cir block is attached to the reset controller, add support >>>>> for de-asserting the reset if a reset controller is specified in dt. >>>>> >>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>> Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> >>>>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>> --- >>>>> .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ >>>>> drivers/media/rc/sunxi-cir.c | 25 >>>>> ++++++++++++++++++++-- >>>>> 2 files changed, 25 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>> b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>> index 23dd5ad..6b70b9b 100644 >>>>> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>> @@ -10,6 +10,7 @@ Required properties: >>>>> >>>>> Optional properties: >>>>> - linux,rc-map-name : Remote control map name. >>>>> +- resets : phandle + reset specifier pair >>>> >>>> >>>> Should it be optional? Or should we use a sun6i compatible with >>>> a mandatory reset phandle? I mean, the driver/hardware is not >>>> going to work with the reset missing on sun6i. >>>> >>>> Seems we are doing it one way for some of our drivers, and >>>> the other (optional) way for more generic ones, like USB. >>> >>> >>> I do not believe that we should add a new compatible just because >>> the reset line of a block is hooked up differently. It is the >>> exact same ip-block. Only now the reset is not controlled >>> through the apb-gate, but controlled separately. >> >> >> He has a point though. Your driver might very well probe nicely and >> everything, but still wouldn't be functional at all because the reset >> line wouldn't have been specified in the DT. > > > Right, just like other drivers we've, see e.g.: > > Documentation/devicetree/bindings/mmc/sunxi-mmc.txt > > Which is dealing with this in the same way. > >> The easiest way to deal with that would be in the bindings doc to >> update it with a compatible for the A31, and mentionning that the >> reset property is mandatory there. > > > No the easiest way to deal with this is to expect people writing > the dts to know what they are doing, just like we do for a lot > of the other blocks in sun6i. > > Maybe put a generic note somewhere that sun6i has a reset controller, > and that for all the blocks with optional resets property it should > be considered mandatory on sun6i ? > > I'm sorry but I'm not going to make this change for the ir bindings > given that we've the same situation in a lot of other places. > > Consistency is important. Moreover I believe that having a sun6i > specific compatible string is just wrong, since it is the exact > same hardware block as on sun5i, just with its reset line routed > differently, just like e.g. the mmc controller, the uarts or the gmac > all of which also do not have a sun6i specific compatible to enforce > reset controller usage. > > Regards, > > Hans > > > > >> Note that the code itself might not change at all though. I'd just >> like to avoid any potential breaking of the DT bindings themselves. If >> we further want to refine the code, we can do that however we want. >> >> I have a slight preference for a clean error if reset is missing, but >> I won't get in the way just for that. Seems this patch and the following patch were overlooked after the discussion. Any chance we could get this in? Hans has made a good argument, so I take back any doubts I raised. ChenYu -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 19-01-15 15:10, Chen-Yu Tsai wrote: > Hi, > > On Sat, Dec 20, 2014 at 6:20 PM, Hans de Goede <hdegoede@redhat.com> wrote: >> Hi, >> >> >> On 19-12-14 19:17, Maxime Ripard wrote: >>> >>> Hi, >>> >>> On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote: >>>> >>>> Hi, >>>> >>>> On 18-12-14 03:48, Chen-Yu Tsai wrote: >>>>> >>>>> Hi, >>>>> >>>>> On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede@redhat.com> >>>>> wrote: >>>>>> >>>>>> On sun6i the cir block is attached to the reset controller, add support >>>>>> for de-asserting the reset if a reset controller is specified in dt. >>>>>> >>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>>> Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> >>>>>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>>> --- >>>>>> .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ >>>>>> drivers/media/rc/sunxi-cir.c | 25 >>>>>> ++++++++++++++++++++-- >>>>>> 2 files changed, 25 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> index 23dd5ad..6b70b9b 100644 >>>>>> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> @@ -10,6 +10,7 @@ Required properties: >>>>>> >>>>>> Optional properties: >>>>>> - linux,rc-map-name : Remote control map name. >>>>>> +- resets : phandle + reset specifier pair >>>>> >>>>> >>>>> Should it be optional? Or should we use a sun6i compatible with >>>>> a mandatory reset phandle? I mean, the driver/hardware is not >>>>> going to work with the reset missing on sun6i. >>>>> >>>>> Seems we are doing it one way for some of our drivers, and >>>>> the other (optional) way for more generic ones, like USB. >>>> >>>> >>>> I do not believe that we should add a new compatible just because >>>> the reset line of a block is hooked up differently. It is the >>>> exact same ip-block. Only now the reset is not controlled >>>> through the apb-gate, but controlled separately. >>> >>> >>> He has a point though. Your driver might very well probe nicely and >>> everything, but still wouldn't be functional at all because the reset >>> line wouldn't have been specified in the DT. >> >> >> Right, just like other drivers we've, see e.g.: >> >> Documentation/devicetree/bindings/mmc/sunxi-mmc.txt >> >> Which is dealing with this in the same way. >> >>> The easiest way to deal with that would be in the bindings doc to >>> update it with a compatible for the A31, and mentionning that the >>> reset property is mandatory there. >> >> >> No the easiest way to deal with this is to expect people writing >> the dts to know what they are doing, just like we do for a lot >> of the other blocks in sun6i. >> >> Maybe put a generic note somewhere that sun6i has a reset controller, >> and that for all the blocks with optional resets property it should >> be considered mandatory on sun6i ? >> >> I'm sorry but I'm not going to make this change for the ir bindings >> given that we've the same situation in a lot of other places. >> >> Consistency is important. Moreover I believe that having a sun6i >> specific compatible string is just wrong, since it is the exact >> same hardware block as on sun5i, just with its reset line routed >> differently, just like e.g. the mmc controller, the uarts or the gmac >> all of which also do not have a sun6i specific compatible to enforce >> reset controller usage. >> >> Regards, >> >> Hans >> >> >> >> >>> Note that the code itself might not change at all though. I'd just >>> like to avoid any potential breaking of the DT bindings themselves. If >>> we further want to refine the code, we can do that however we want. >>> >>> I have a slight preference for a clean error if reset is missing, but >>> I won't get in the way just for that. > > Seems this patch and the following patch were overlooked after the > discussion. Any chance we could get this in? I'm a linux/media sub-maintainer, so I've already send a pull-req for these 2 to the linux/media maintainer, iow this is taken care of :) Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 19, 2015 at 10:17 PM, Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > > On 19-01-15 15:10, Chen-Yu Tsai wrote: >> >> Hi, >> >> On Sat, Dec 20, 2014 at 6:20 PM, Hans de Goede <hdegoede@redhat.com> >> wrote: >>> >>> Hi, >>> >>> >>> On 19-12-14 19:17, Maxime Ripard wrote: >>>> >>>> >>>> Hi, >>>> >>>> On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote: >>>>> >>>>> >>>>> Hi, >>>>> >>>>> On 18-12-14 03:48, Chen-Yu Tsai wrote: >>>>>> >>>>>> >>>>>> Hi, >>>>>> >>>>>> On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede <hdegoede@redhat.com> >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> On sun6i the cir block is attached to the reset controller, add >>>>>>> support >>>>>>> for de-asserting the reset if a reset controller is specified in dt. >>>>>>> >>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>>>>> Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> >>>>>>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com> >>>>>>> --- >>>>>>> .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ >>>>>>> drivers/media/rc/sunxi-cir.c | 25 >>>>>>> ++++++++++++++++++++-- >>>>>>> 2 files changed, 25 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>>> b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>>> index 23dd5ad..6b70b9b 100644 >>>>>>> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>>> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>>> @@ -10,6 +10,7 @@ Required properties: >>>>>>> >>>>>>> Optional properties: >>>>>>> - linux,rc-map-name : Remote control map name. >>>>>>> +- resets : phandle + reset specifier pair >>>>>> >>>>>> >>>>>> >>>>>> Should it be optional? Or should we use a sun6i compatible with >>>>>> a mandatory reset phandle? I mean, the driver/hardware is not >>>>>> going to work with the reset missing on sun6i. >>>>>> >>>>>> Seems we are doing it one way for some of our drivers, and >>>>>> the other (optional) way for more generic ones, like USB. >>>>> >>>>> >>>>> >>>>> I do not believe that we should add a new compatible just because >>>>> the reset line of a block is hooked up differently. It is the >>>>> exact same ip-block. Only now the reset is not controlled >>>>> through the apb-gate, but controlled separately. >>>> >>>> >>>> >>>> He has a point though. Your driver might very well probe nicely and >>>> everything, but still wouldn't be functional at all because the reset >>>> line wouldn't have been specified in the DT. >>> >>> >>> >>> Right, just like other drivers we've, see e.g.: >>> >>> Documentation/devicetree/bindings/mmc/sunxi-mmc.txt >>> >>> Which is dealing with this in the same way. >>> >>>> The easiest way to deal with that would be in the bindings doc to >>>> update it with a compatible for the A31, and mentionning that the >>>> reset property is mandatory there. >>> >>> >>> >>> No the easiest way to deal with this is to expect people writing >>> the dts to know what they are doing, just like we do for a lot >>> of the other blocks in sun6i. >>> >>> Maybe put a generic note somewhere that sun6i has a reset controller, >>> and that for all the blocks with optional resets property it should >>> be considered mandatory on sun6i ? >>> >>> I'm sorry but I'm not going to make this change for the ir bindings >>> given that we've the same situation in a lot of other places. >>> >>> Consistency is important. Moreover I believe that having a sun6i >>> specific compatible string is just wrong, since it is the exact >>> same hardware block as on sun5i, just with its reset line routed >>> differently, just like e.g. the mmc controller, the uarts or the gmac >>> all of which also do not have a sun6i specific compatible to enforce >>> reset controller usage. >>> >>> Regards, >>> >>> Hans >>> >>> >>> >>> >>>> Note that the code itself might not change at all though. I'd just >>>> like to avoid any potential breaking of the DT bindings themselves. If >>>> we further want to refine the code, we can do that however we want. >>>> >>>> I have a slight preference for a clean error if reset is missing, but >>>> I won't get in the way just for that. >> >> >> Seems this patch and the following patch were overlooked after the >> discussion. Any chance we could get this in? > > > I'm a linux/media sub-maintainer, so I've already send a pull-req for > these 2 to the linux/media maintainer, iow this is taken care of :) That's good to hear. I was going through the mainlining effort page, and couldn't find these 2 in linux-next. I'll mark them as planned for 3.20 then. Chen-Yu -- To unsubscribe from this list: send the line "unsubscribe linux-media" 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/media/sunxi-ir.txt b/Documentation/devicetree/bindings/media/sunxi-ir.txt index 23dd5ad..6b70b9b 100644 --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt @@ -10,6 +10,7 @@ Required properties: Optional properties: - linux,rc-map-name : Remote control map name. +- resets : phandle + reset specifier pair Example: @@ -17,6 +18,7 @@ ir0: ir@01c21800 { compatible = "allwinner,sun4i-a10-ir"; clocks = <&apb0_gates 6>, <&ir0_clk>; clock-names = "apb", "ir"; + resets = <&apb0_rst 1>; interrupts = <0 5 1>; reg = <0x01C21800 0x40>; linux,rc-map-name = "rc-rc6-mce"; diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c index 340f7f5..06170e0 100644 --- a/drivers/media/rc/sunxi-cir.c +++ b/drivers/media/rc/sunxi-cir.c @@ -23,6 +23,7 @@ #include <linux/interrupt.h> #include <linux/module.h> #include <linux/of_platform.h> +#include <linux/reset.h> #include <media/rc-core.h> #define SUNXI_IR_DEV "sunxi-ir" @@ -95,6 +96,7 @@ struct sunxi_ir { int irq; struct clk *clk; struct clk *apb_clk; + struct reset_control *rst; const char *map_name; }; @@ -166,15 +168,29 @@ static int sunxi_ir_probe(struct platform_device *pdev) return PTR_ERR(ir->clk); } + /* Reset (optional) */ + ir->rst = devm_reset_control_get_optional(dev, NULL); + if (IS_ERR(ir->rst)) { + ret = PTR_ERR(ir->rst); + if (ret == -EPROBE_DEFER) + return ret; + ir->rst = NULL; + } else { + ret = reset_control_deassert(ir->rst); + if (ret) + return ret; + } + ret = clk_set_rate(ir->clk, SUNXI_IR_BASE_CLK); if (ret) { dev_err(dev, "set ir base clock failed!\n"); - return ret; + goto exit_reset_assert; } if (clk_prepare_enable(ir->apb_clk)) { dev_err(dev, "try to enable apb_ir_clk failed\n"); - return -EINVAL; + ret = -EINVAL; + goto exit_reset_assert; } if (clk_prepare_enable(ir->clk)) { @@ -271,6 +287,9 @@ exit_clkdisable_clk: clk_disable_unprepare(ir->clk); exit_clkdisable_apb_clk: clk_disable_unprepare(ir->apb_clk); +exit_reset_assert: + if (ir->rst) + reset_control_assert(ir->rst); return ret; } @@ -282,6 +301,8 @@ static int sunxi_ir_remove(struct platform_device *pdev) clk_disable_unprepare(ir->clk); clk_disable_unprepare(ir->apb_clk); + if (ir->rst) + reset_control_assert(ir->rst); spin_lock_irqsave(&ir->ir_lock, flags); /* disable IR IRQ */