Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
> > I think there is something wrong with an interface that makes you use > > IS_ERR_OR_NULL(). If you are calling reset_control_get_optional(), that' > > should not return an error when there is no reset controller listed > > in the device tree. We should still have a way to propagate -EPROBE_DEFER, > > or bail out if there is a reset controller but there is something wrong > > with it, but otherwise I'd suggest just leaving NULL as a valid pointer > > in drv_data->rstc and making sure that the reset controller functions > > can just deal with a NULL argument, so you never have to check it again. > > Actually, it's not the reset framework but the driver itself that > needs this. The framework will always return an error pointer here, > but we won't ever call reset_control_get_optional if we are not probed > with DT, and in that case, we will have NULL is data->rstc, hence why > we need to use IS_ERR_OR_NULL. > > We should probably fix the reset functions, but maybe that can come > later so that we have marvell's defconfig fixed? Yes, let's fix that incrementally. signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
I think there is something wrong with an interface that makes you use IS_ERR_OR_NULL(). If you are calling reset_control_get_optional(), that' should not return an error when there is no reset controller listed in the device tree. We should still have a way to propagate -EPROBE_DEFER, or bail out if there is a reset controller but there is something wrong with it, but otherwise I'd suggest just leaving NULL as a valid pointer in drv_data-rstc and making sure that the reset controller functions can just deal with a NULL argument, so you never have to check it again. Actually, it's not the reset framework but the driver itself that needs this. The framework will always return an error pointer here, but we won't ever call reset_control_get_optional if we are not probed with DT, and in that case, we will have NULL is data-rstc, hence why we need to use IS_ERR_OR_NULL. We should probably fix the reset functions, but maybe that can come later so that we have marvell's defconfig fixed? Yes, let's fix that incrementally. signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
Arnd, Olof, Kevin, On 24/03/2014 14:33, Wolfram Sang wrote: > >>> Looping linux-next into the CC since this is the cause of the failure >>> in orion5x_defconfig there, and no point in anyone else re-doing the >>> same bisect. >> >> I sent a fix for this that hasn't been picked up yet: >> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html >> >> IIRC, Wolfram's away until Monday, so I guess it will be merged some >> time next week. > > I'd love to apply the fix but the reset framework update is not in > linux-next yet. > I saw the there was a pull request resent last week. Is there any reason to not have pulled it, yet? http://www.spinics.net/lists/arm-kernel/msg316518.html I would really like that it was sorted out, because all the build using the mvebu related defconfig fails since two weeks because of it now. I am ready to help if you need it. Thanks! -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
> > Looping linux-next into the CC since this is the cause of the failure > > in orion5x_defconfig there, and no point in anyone else re-doing the > > same bisect. > > I sent a fix for this that hasn't been picked up yet: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html > > IIRC, Wolfram's away until Monday, so I guess it will be merged some > time next week. I'd love to apply the fix but the reset framework update is not in linux-next yet. signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Sat, Mar 22, 2014 at 12:11:24PM +0100, Arnd Bergmann wrote: > On Friday 21 March 2014 20:17:39 Maxime Ripard wrote: > > On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: > > > On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux > > > wrote: > > > > On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: > > > >> On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux > > > >> wrote: > > > >> > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > > >> > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > > >> > > exit_free_irq: > > > >> > > free_irq(drv_data->irq, drv_data); > > > >> > > exit_reset: > > > >> > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > > >> > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > > >> > > + !IS_ERR(drv_data->rstc)) > > > >> > > reset_control_assert(drv_data->rstc); > > > >> > > > > >> > Another question is... why do we need to check pd->dev.of_node here? > > > >> > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > > > >> > controller node, so drv_data->rstc is either going to be a valid > > > >> > pointer, or it's going to be an error pointer - neither > > > >> > reset_control_get() nor devm_reset_control_get return NULL. > > > >> > > > >> Following back on this as I was doing the patch, actually, > > > >> drv_data->rstc will be NULL if we're not probed by DT, and hence never > > > >> call reset_control_get, that would set an error pointer. > > > >> > > > >> But then, we can use IS_ERR_OR_NULL on drv_data->rstc. > > > > > > > > I think you can also move the devm_reset_control_get() into the main > > > > probe function: you're only checking for -EPROBE_DEFER from it to fail, > > > > allowing other errors to continue with the driver init. This means > > > > that on non-OF, devm_reset_control_get() will fail with -ENOENT. > > > > > > Looping linux-next into the CC since this is the cause of the failure > > > in orion5x_defconfig there, and no point in anyone else re-doing the > > > same bisect. > > > > I sent a fix for this that hasn't been picked up yet: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html > > > > IIRC, Wolfram's away until Monday, so I guess it will be merged some > > time next week. > > I think there is something wrong with an interface that makes you use > IS_ERR_OR_NULL(). If you are calling reset_control_get_optional(), that' > should not return an error when there is no reset controller listed > in the device tree. We should still have a way to propagate -EPROBE_DEFER, > or bail out if there is a reset controller but there is something wrong > with it, but otherwise I'd suggest just leaving NULL as a valid pointer > in drv_data->rstc and making sure that the reset controller functions > can just deal with a NULL argument, so you never have to check it again. Actually, it's not the reset framework but the driver itself that needs this. The framework will always return an error pointer here, but we won't ever call reset_control_get_optional if we are not probed with DT, and in that case, we will have NULL is data->rstc, hence why we need to use IS_ERR_OR_NULL. We should probably fix the reset functions, but maybe that can come later so that we have marvell's defconfig fixed? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Sat, Mar 22, 2014 at 12:11:24PM +0100, Arnd Bergmann wrote: On Friday 21 March 2014 20:17:39 Maxime Ripard wrote: On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data-rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data-rstc. I think you can also move the devm_reset_control_get() into the main probe function: you're only checking for -EPROBE_DEFER from it to fail, allowing other errors to continue with the driver init. This means that on non-OF, devm_reset_control_get() will fail with -ENOENT. Looping linux-next into the CC since this is the cause of the failure in orion5x_defconfig there, and no point in anyone else re-doing the same bisect. I sent a fix for this that hasn't been picked up yet: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html IIRC, Wolfram's away until Monday, so I guess it will be merged some time next week. I think there is something wrong with an interface that makes you use IS_ERR_OR_NULL(). If you are calling reset_control_get_optional(), that' should not return an error when there is no reset controller listed in the device tree. We should still have a way to propagate -EPROBE_DEFER, or bail out if there is a reset controller but there is something wrong with it, but otherwise I'd suggest just leaving NULL as a valid pointer in drv_data-rstc and making sure that the reset controller functions can just deal with a NULL argument, so you never have to check it again. Actually, it's not the reset framework but the driver itself that needs this. The framework will always return an error pointer here, but we won't ever call reset_control_get_optional if we are not probed with DT, and in that case, we will have NULL is data-rstc, hence why we need to use IS_ERR_OR_NULL. We should probably fix the reset functions, but maybe that can come later so that we have marvell's defconfig fixed? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
Looping linux-next into the CC since this is the cause of the failure in orion5x_defconfig there, and no point in anyone else re-doing the same bisect. I sent a fix for this that hasn't been picked up yet: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html IIRC, Wolfram's away until Monday, so I guess it will be merged some time next week. I'd love to apply the fix but the reset framework update is not in linux-next yet. signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
Arnd, Olof, Kevin, On 24/03/2014 14:33, Wolfram Sang wrote: Looping linux-next into the CC since this is the cause of the failure in orion5x_defconfig there, and no point in anyone else re-doing the same bisect. I sent a fix for this that hasn't been picked up yet: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html IIRC, Wolfram's away until Monday, so I guess it will be merged some time next week. I'd love to apply the fix but the reset framework update is not in linux-next yet. I saw the there was a pull request resent last week. Is there any reason to not have pulled it, yet? http://www.spinics.net/lists/arm-kernel/msg316518.html I would really like that it was sorted out, because all the build using the mvebu related defconfig fails since two weeks because of it now. I am ready to help if you need it. Thanks! -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Friday 21 March 2014 20:17:39 Maxime Ripard wrote: > On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: > > On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux > > wrote: > > > On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: > > >> On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > > >> > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > >> > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > >> > > exit_free_irq: > > >> > > free_irq(drv_data->irq, drv_data); > > >> > > exit_reset: > > >> > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > >> > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > >> > > + !IS_ERR(drv_data->rstc)) > > >> > > reset_control_assert(drv_data->rstc); > > >> > > > >> > Another question is... why do we need to check pd->dev.of_node here? > > >> > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > > >> > controller node, so drv_data->rstc is either going to be a valid > > >> > pointer, or it's going to be an error pointer - neither > > >> > reset_control_get() nor devm_reset_control_get return NULL. > > >> > > >> Following back on this as I was doing the patch, actually, > > >> drv_data->rstc will be NULL if we're not probed by DT, and hence never > > >> call reset_control_get, that would set an error pointer. > > >> > > >> But then, we can use IS_ERR_OR_NULL on drv_data->rstc. > > > > > > I think you can also move the devm_reset_control_get() into the main > > > probe function: you're only checking for -EPROBE_DEFER from it to fail, > > > allowing other errors to continue with the driver init. This means > > > that on non-OF, devm_reset_control_get() will fail with -ENOENT. > > > > Looping linux-next into the CC since this is the cause of the failure > > in orion5x_defconfig there, and no point in anyone else re-doing the > > same bisect. > > I sent a fix for this that hasn't been picked up yet: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html > > IIRC, Wolfram's away until Monday, so I guess it will be merged some > time next week. I think there is something wrong with an interface that makes you use IS_ERR_OR_NULL(). If you are calling reset_control_get_optional(), that' should not return an error when there is no reset controller listed in the device tree. We should still have a way to propagate -EPROBE_DEFER, or bail out if there is a reset controller but there is something wrong with it, but otherwise I'd suggest just leaving NULL as a valid pointer in drv_data->rstc and making sure that the reset controller functions can just deal with a NULL argument, so you never have to check it again. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: > On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux > wrote: > > On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: > >> On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > >> > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > >> > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > >> > > exit_free_irq: > >> > > free_irq(drv_data->irq, drv_data); > >> > > exit_reset: > >> > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > >> > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > >> > > + !IS_ERR(drv_data->rstc)) > >> > > reset_control_assert(drv_data->rstc); > >> > > >> > Another question is... why do we need to check pd->dev.of_node here? > >> > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > >> > controller node, so drv_data->rstc is either going to be a valid > >> > pointer, or it's going to be an error pointer - neither > >> > reset_control_get() nor devm_reset_control_get return NULL. > >> > >> Following back on this as I was doing the patch, actually, > >> drv_data->rstc will be NULL if we're not probed by DT, and hence never > >> call reset_control_get, that would set an error pointer. > >> > >> But then, we can use IS_ERR_OR_NULL on drv_data->rstc. > > > > I think you can also move the devm_reset_control_get() into the main > > probe function: you're only checking for -EPROBE_DEFER from it to fail, > > allowing other errors to continue with the driver init. This means > > that on non-OF, devm_reset_control_get() will fail with -ENOENT. > > Looping linux-next into the CC since this is the cause of the failure > in orion5x_defconfig there, and no point in anyone else re-doing the > same bisect. I sent a fix for this that hasn't been picked up yet: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html IIRC, Wolfram's away until Monday, so I guess it will be merged some time next week. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data-rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data-rstc. I think you can also move the devm_reset_control_get() into the main probe function: you're only checking for -EPROBE_DEFER from it to fail, allowing other errors to continue with the driver init. This means that on non-OF, devm_reset_control_get() will fail with -ENOENT. Looping linux-next into the CC since this is the cause of the failure in orion5x_defconfig there, and no point in anyone else re-doing the same bisect. I sent a fix for this that hasn't been picked up yet: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html IIRC, Wolfram's away until Monday, so I guess it will be merged some time next week. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Friday 21 March 2014 20:17:39 Maxime Ripard wrote: On Fri, Mar 21, 2014 at 11:49:59AM -0400, Paul Gortmaker wrote: On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data-rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data-rstc. I think you can also move the devm_reset_control_get() into the main probe function: you're only checking for -EPROBE_DEFER from it to fail, allowing other errors to continue with the driver init. This means that on non-OF, devm_reset_control_get() will fail with -ENOENT. Looping linux-next into the CC since this is the cause of the failure in orion5x_defconfig there, and no point in anyone else re-doing the same bisect. I sent a fix for this that hasn't been picked up yet: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/239069.html IIRC, Wolfram's away until Monday, so I guess it will be merged some time next week. I think there is something wrong with an interface that makes you use IS_ERR_OR_NULL(). If you are calling reset_control_get_optional(), that' should not return an error when there is no reset controller listed in the device tree. We should still have a way to propagate -EPROBE_DEFER, or bail out if there is a reset controller but there is something wrong with it, but otherwise I'd suggest just leaving NULL as a valid pointer in drv_data-rstc and making sure that the reset controller functions can just deal with a NULL argument, so you never have to check it again. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux
wrote:
> On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote:
>> On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote:
>> > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote:
>> > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>> > > exit_free_irq:
>> > > free_irq(drv_data->irq, drv_data);
>> > > exit_reset:
>> > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc))
>> > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) &&
>> > > + !IS_ERR(drv_data->rstc))
>> > > reset_control_assert(drv_data->rstc);
>> >
>> > Another question is... why do we need to check pd->dev.of_node here?
>> > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset
>> > controller node, so drv_data->rstc is either going to be a valid
>> > pointer, or it's going to be an error pointer - neither
>> > reset_control_get() nor devm_reset_control_get return NULL.
>>
>> Following back on this as I was doing the patch, actually,
>> drv_data->rstc will be NULL if we're not probed by DT, and hence never
>> call reset_control_get, that would set an error pointer.
>>
>> But then, we can use IS_ERR_OR_NULL on drv_data->rstc.
>
> I think you can also move the devm_reset_control_get() into the main
> probe function: you're only checking for -EPROBE_DEFER from it to fail,
> allowing other errors to continue with the driver init. This means
> that on non-OF, devm_reset_control_get() will fail with -ENOENT.
Looping linux-next into the CC since this is the cause of the failure
in orion5x_defconfig there, and no point in anyone else re-doing the
same bisect.
Paul.
--
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[80c69915e5fbe6493119d87eee2a2a6a7115c74c] i2c: mv64xxx: fix circular
Kconfig dependency
running ./x
#
# configuration written to .config
#
drivers/built-in.o: In function `mv64xxx_i2c_remove':
/home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:924:
undefined reference to `reset_control_assert'
drivers/built-in.o: In function `mv64xxx_i2c_probe':
/home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:904:
undefined reference to `reset_control_assert'
drivers/built-in.o: In function `mv64xxx_of_config':
/home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:771:
undefined reference to `devm_reset_control_get'
/home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:778:
undefined reference to `reset_control_deassert'
make: *** [vmlinux] Error 1
80c69915e5fbe6493119d87eee2a2a6a7115c74c is the first bad commit
commit 80c69915e5fbe6493119d87eee2a2a6a7115c74c
Author: Wolfram Sang
Date: Thu Mar 6 10:08:50 2014 +0100
i2c: mv64xxx: fix circular Kconfig dependency
Commit 370136bc67c3 ("i2c: mv64xxx: Add reset deassert call")
introduced:
drivers/video/Kconfig:42:error: recursive dependency detected!
ARCH_SUNXI selects RESET_CONTROLLER anyhow.
Signed-off-by: Wolfram Sang
:04 04 533a0ca6b40f2dd1d0b3bb434e6ed13ff4796953
cfd47b9ad19651148a2d0d4fa3a4df0b8cbbe1df Mdrivers
bisect run success
>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Mon, Mar 10, 2014 at 7:29 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data-rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data-rstc. I think you can also move the devm_reset_control_get() into the main probe function: you're only checking for -EPROBE_DEFER from it to fail, allowing other errors to continue with the driver init. This means that on non-OF, devm_reset_control_get() will fail with -ENOENT. Looping linux-next into the CC since this is the cause of the failure in orion5x_defconfig there, and no point in anyone else re-doing the same bisect. Paul. -- Bisecting: 0 revisions left to test after this (roughly 0 steps) [80c69915e5fbe6493119d87eee2a2a6a7115c74c] i2c: mv64xxx: fix circular Kconfig dependency running ./x # # configuration written to .config # drivers/built-in.o: In function `mv64xxx_i2c_remove': /home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:924: undefined reference to `reset_control_assert' drivers/built-in.o: In function `mv64xxx_i2c_probe': /home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:904: undefined reference to `reset_control_assert' drivers/built-in.o: In function `mv64xxx_of_config': /home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:771: undefined reference to `devm_reset_control_get' /home/paul/git/linux-head/drivers/i2c/busses/i2c-mv64xxx.c:778: undefined reference to `reset_control_deassert' make: *** [vmlinux] Error 1 80c69915e5fbe6493119d87eee2a2a6a7115c74c is the first bad commit commit 80c69915e5fbe6493119d87eee2a2a6a7115c74c Author: Wolfram Sang w...@the-dreams.de Date: Thu Mar 6 10:08:50 2014 +0100 i2c: mv64xxx: fix circular Kconfig dependency Commit 370136bc67c3 (i2c: mv64xxx: Add reset deassert call) introduced: drivers/video/Kconfig:42:error: recursive dependency detected! ARCH_SUNXI selects RESET_CONTROLLER anyhow. Signed-off-by: Wolfram Sang w...@the-dreams.de :04 04 533a0ca6b40f2dd1d0b3bb434e6ed13ff4796953 cfd47b9ad19651148a2d0d4fa3a4df0b8cbbe1df Mdrivers bisect run success -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: > On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > > exit_free_irq: > > > free_irq(drv_data->irq, drv_data); > > > exit_reset: > > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > > + !IS_ERR(drv_data->rstc)) > > > reset_control_assert(drv_data->rstc); > > > > Another question is... why do we need to check pd->dev.of_node here? > > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > > controller node, so drv_data->rstc is either going to be a valid > > pointer, or it's going to be an error pointer - neither > > reset_control_get() nor devm_reset_control_get return NULL. > > Following back on this as I was doing the patch, actually, > drv_data->rstc will be NULL if we're not probed by DT, and hence never > call reset_control_get, that would set an error pointer. > > But then, we can use IS_ERR_OR_NULL on drv_data->rstc. I think you can also move the devm_reset_control_get() into the main probe function: you're only checking for -EPROBE_DEFER from it to fail, allowing other errors to continue with the driver init. This means that on non-OF, devm_reset_control_get() will fail with -ENOENT. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > exit_free_irq: > > free_irq(drv_data->irq, drv_data); > > exit_reset: > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > + !IS_ERR(drv_data->rstc)) > > reset_control_assert(drv_data->rstc); > > Another question is... why do we need to check pd->dev.of_node here? > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > controller node, so drv_data->rstc is either going to be a valid > pointer, or it's going to be an error pointer - neither > reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data->rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data->rstc. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data-rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data-rstc. -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Mon, Mar 10, 2014 at 11:58:08AM +0100, Maxime Ripard wrote: On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Following back on this as I was doing the patch, actually, drv_data-rstc will be NULL if we're not probed by DT, and hence never call reset_control_get, that would set an error pointer. But then, we can use IS_ERR_OR_NULL on drv_data-rstc. I think you can also move the devm_reset_control_get() into the main probe function: you're only checking for -EPROBE_DEFER from it to fail, allowing other errors to continue with the driver init. This means that on non-OF, devm_reset_control_get() will fail with -ENOENT. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 06:29:49PM +0100, Wolfram Sang wrote: > > > > Another question is... why do we need to check pd->dev.of_node here? > > > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > > > controller node, so drv_data->rstc is either going to be a valid > > > pointer, or it's going to be an error pointer - neither > > > reset_control_get() nor devm_reset_control_get return NULL. > > > > Hmmm, right. I'll fix this in a later version. > > > > Wolfram, do you want me to respin the patch making use of > > reset_get_optional introduced by Philip in its other mail? > > I think I'd prefer both issues fixed with one patch like in "fixing up > reset controller handling". You mean the of_node check and the use of reset_control_get_optional, right? > And you might want to give a Tested- or Reviewed-by tag to Philipp's > patch if you are going to use it. Yes, I will. I'll only have access to the hardware on monday though, so I won't be able to actually test it before then. Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
> > Another question is... why do we need to check pd->dev.of_node here? > > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > > controller node, so drv_data->rstc is either going to be a valid > > pointer, or it's going to be an error pointer - neither > > reset_control_get() nor devm_reset_control_get return NULL. > > Hmmm, right. I'll fix this in a later version. > > Wolfram, do you want me to respin the patch making use of > reset_get_optional introduced by Philip in its other mail? I think I'd prefer both issues fixed with one patch like in "fixing up reset controller handling". And you might want to give a Tested- or Reviewed-by tag to Philipp's patch if you are going to use it. signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: > On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > > exit_free_irq: > > free_irq(drv_data->irq, drv_data); > > exit_reset: > > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > > + !IS_ERR(drv_data->rstc)) > > reset_control_assert(drv_data->rstc); > > Another question is... why do we need to check pd->dev.of_node here? > If CONFIG_RESET_CONTROLLER is set, we always try to get the reset > controller node, so drv_data->rstc is either going to be a valid > pointer, or it's going to be an error pointer - neither > reset_control_get() nor devm_reset_control_get return NULL. Hmmm, right. I'll fix this in a later version. Wolfram, do you want me to respin the patch making use of reset_get_optional introduced by Philip in its other mail? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: > @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) > exit_free_irq: > free_irq(drv_data->irq, drv_data); > exit_reset: > - if (pd->dev.of_node && !IS_ERR(drv_data->rstc)) > + if (pd->dev.of_node && IS_ENABLED(CONFIG_RESET_CONTROLLER) && > + !IS_ERR(drv_data->rstc)) > reset_control_assert(drv_data->rstc); Another question is... why do we need to check pd->dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data->rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 04:08:36PM +, Russell King - ARM Linux wrote: On Fri, Mar 07, 2014 at 03:59:30PM +0100, Maxime Ripard wrote: @@ -900,7 +902,8 @@ mv64xxx_i2c_probe(struct platform_device *pd) exit_free_irq: free_irq(drv_data-irq, drv_data); exit_reset: - if (pd-dev.of_node !IS_ERR(drv_data-rstc)) + if (pd-dev.of_node IS_ENABLED(CONFIG_RESET_CONTROLLER) + !IS_ERR(drv_data-rstc)) reset_control_assert(drv_data-rstc); Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Hmmm, right. I'll fix this in a later version. Wolfram, do you want me to respin the patch making use of reset_get_optional introduced by Philip in its other mail? -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Hmmm, right. I'll fix this in a later version. Wolfram, do you want me to respin the patch making use of reset_get_optional introduced by Philip in its other mail? I think I'd prefer both issues fixed with one patch like in fixing up reset controller handling. And you might want to give a Tested- or Reviewed-by tag to Philipp's patch if you are going to use it. signature.asc Description: Digital signature
Re: [PATCH] i2c: mv64xxx: Fix compilation breakage
On Fri, Mar 07, 2014 at 06:29:49PM +0100, Wolfram Sang wrote: Another question is... why do we need to check pd-dev.of_node here? If CONFIG_RESET_CONTROLLER is set, we always try to get the reset controller node, so drv_data-rstc is either going to be a valid pointer, or it's going to be an error pointer - neither reset_control_get() nor devm_reset_control_get return NULL. Hmmm, right. I'll fix this in a later version. Wolfram, do you want me to respin the patch making use of reset_get_optional introduced by Philip in its other mail? I think I'd prefer both issues fixed with one patch like in fixing up reset controller handling. You mean the of_node check and the use of reset_control_get_optional, right? And you might want to give a Tested- or Reviewed-by tag to Philipp's patch if you are going to use it. Yes, I will. I'll only have access to the hardware on monday though, so I won't be able to actually test it before then. Thanks, Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
