Re: Reset implementation for Zynq

Re: Reset implementation for Zynq

[Moritz Fischer]

Iztok,

On Fri, Oct 21, 2016 at 03:08:47AM -0700, iztok.jeras@redpitaya.com wrote:
> Hi Moritz,
>  
> I was looking at your reset implementation for Zynq:
> https://github.com/Xilinx/linux-xlnx/blob/629041605b93343ad2e8971ceaac3edcef0b043b/drivers/reset/reset-zynq.c
> I went through related mailing list posts (including earlier versions of the patch) so I kind of understand what to change in the device tree.

Please look at the upstream kernel sources and use the mailing list
(lkml) if you want to report bugs. Xilinx' vendor tree might or might
not be up to date.

> I would like to use this driver to reset the Zynq I2C controller, since we have trouble with it getting into a lock up state.
> I plan to use function device_reset_optional() from:
> https://github.com/Xilinx/linux-xlnx/blob/629041605b93343ad2e8971ceaac3edcef0b043b/include/linux/reset.h
>  
> But this function is calling the reset function pointer from the reset_control_ops structure.
> For the zynq driver this function pointer is not defined, only assert, deassert and status are.
>  
> Is this a missing implementation, or is there a default implementation (I did not find one) which which performs an assert+deassert,
> or is there another set of reset APIs I should use inside the kernel.

You could just call reset_control_assert() and reset_control_deassert().
You're right there is currently no implementation for the 'reset' function for
zynq (and most of the other SoCs). I'll need to see if it makes sense at
all.

Please note that you'd probably have to modify the i2c driver to
integrate reset functionality cleanly.

Thanks,

Moritz

Re: Reset implementation for Zynq

[Philipp Zabel]

Hi Iztok, Moritz,

Am Freitag, den 21.10.2016, 10:04 -0700 schrieb Moritz Fischer:
> Iztok,
> 
> On Fri, Oct 21, 2016 at 03:08:47AM -0700, iztok.jeras@redpitaya.com wrote:
> > Hi Moritz,
> >  
> > I was looking at your reset implementation for Zynq:
> > https://github.com/Xilinx/linux-xlnx/blob/629041605b93343ad2e8971ceaac3edcef0b043b/drivers/reset/reset-zynq.c
> > I went through related mailing list posts (including earlier versions of the patch) so I kind of understand what to change in the device tree.
> 
> Please look at the upstream kernel sources and use the mailing list
> (lkml) if you want to report bugs. Xilinx' vendor tree might or might
> not be up to date.
> 
> > I would like to use this driver to reset the Zynq I2C controller, since we have trouble with it getting into a lock up state.
> > I plan to use function device_reset_optional() from:
> > https://github.com/Xilinx/linux-xlnx/blob/629041605b93343ad2e8971ceaac3edcef0b043b/include/linux/reset.h
> >  
> > But this function is calling the reset function pointer from the reset_control_ops structure.
> > For the zynq driver this function pointer is not defined, only assert, deassert and status are.
> >  
> > Is this a missing implementation, or is there a default implementation (I did not find one) which which performs an assert+deassert,
> > or is there another set of reset APIs I should use inside the kernel.
> 
> You could just call reset_control_assert() and reset_control_deassert().
> You're right there is currently no implementation for the 'reset' function for
> zynq (and most of the other SoCs). I'll need to see if it makes sense at
> all.

The implementation of reset_control_reset in software really only makes
sense if the reset provider driver knows about the necessary delays for
all reset consumers.

> Please note that you'd probably have to modify the i2c driver to
> integrate reset functionality cleanly.
> 
> Thanks,
> 
> Moritz

regards
Philipp

Re: Reset implementation for Zynq

[Moritz Fischer]

Philip,

On Mon, Oct 24, 2016 at 11:00:05AM +0200, Philipp Zabel wrote:
> Hi Iztok, Moritz,
> 
> Am Freitag, den 21.10.2016, 10:04 -0700 schrieb Moritz Fischer:
> > Iztok,
> > 
> > On Fri, Oct 21, 2016 at 03:08:47AM -0700, iztok.jeras@redpitaya.com wrote:
> > > Hi Moritz,
> > >  
> > > I was looking at your reset implementation for Zynq:
> > > https://github.com/Xilinx/linux-xlnx/blob/629041605b93343ad2e8971ceaac3edcef0b043b/drivers/reset/reset-zynq.c
> > > I went through related mailing list posts (including earlier versions of the patch) so I kind of understand what to change in the device tree.
> > 
> > Please look at the upstream kernel sources and use the mailing list
> > (lkml) if you want to report bugs. Xilinx' vendor tree might or might
> > not be up to date.
> > 
> > > I would like to use this driver to reset the Zynq I2C controller, since we have trouble with it getting into a lock up state.
> > > I plan to use function device_reset_optional() from:
> > > https://github.com/Xilinx/linux-xlnx/blob/629041605b93343ad2e8971ceaac3edcef0b043b/include/linux/reset.h
> > >  
> > > But this function is calling the reset function pointer from the reset_control_ops structure.
> > > For the zynq driver this function pointer is not defined, only assert, deassert and status are.
> > >  
> > > Is this a missing implementation, or is there a default implementation (I did not find one) which which performs an assert+deassert,
> > > or is there another set of reset APIs I should use inside the kernel.
> > 
> > You could just call reset_control_assert() and reset_control_deassert().
> > You're right there is currently no implementation for the 'reset' function for
> > zynq (and most of the other SoCs). I'll need to see if it makes sense at
> > all.
> 
> The implementation of reset_control_reset in software really only makes
> sense if the reset provider driver knows about the necessary delays for
> all reset consumers.

That's what I meant by needing to see if it makes sense at all; it makes
no sense to have a 'reset' if you don't know how long it needs to be
asserted for, since that's obviously a consumer property.

Thanks for clarifying,

Moritz

Re: Reset implementation for Zynq

[Shubhrajyoti Datta]

On Fri, Oct 21, 2016 at 10:34 PM, Moritz Fischer
<moritz.fischer@ettus.com> wrote:
> Iztok,
>
> On Fri, Oct 21, 2016 at 03:08:47AM -0700, iztok.jeras@redpitaya.com wrote:
>> Hi Moritz,
>>
>> I was looking at your reset implementation for Zynq:
>> https://github.com/Xilinx/linux-xlnx/blob/629041605b93343ad2e8971ceaac3edcef0b043b/drivers/reset/reset-zynq.c
>> I went through related mailing list posts (including earlier versions of the patch) so I kind of understand what to change in the device tree.
>
> Please look at the upstream kernel sources and use the mailing list
> (lkml) if you want to report bugs. Xilinx' vendor tree might or might
> not be up to date.
>
>> I would like to use this driver to reset the Zynq I2C controller, since we have trouble with it getting into a lock up state.

Can you explain what you are facing and what is meant by lockup state.

Re: Re: Reset implementation for Zynq

[Shubhrajyoti Datta]

Hi Iztok,



On Tue, Nov 8, 2016 at 3:10 PM,  <iztok.jeras@redpitaya.com> wrote:
> Hi Shubhrajyoti,
>
> By lockup state, I mean an undefined/unhandled state of the HW logic state
> machine,
> or something similar in the software driver.
> Based on symptoms I assume it is a HW lockup.
> In practice asking the driver for a new I2C transfer would result in
> nothing happening on the I2C bus (signals remain in hi-Z state).
> Driver or userspace tool reports a transfer error, but I did not copy it.
> And I did not check the state of the driver.
>
> To enter this mode it was enough to connect an oscilloscope probe to the SDA
> signal.
> The capacitance of the probe causes a deep voltage fall, which is quickly
> corrected by the pullup.
> I did not time the glitch, and the shape was partially defined by an I2C
> level shifter.
> The issue can be repeated with about 80% probability.
>
> There are two ways to get back to a working state:
> 1. create a longer zero pulse (about 50ms manual grounding) on SDA
> 2. reset the HW, this was done with a patched driver.
Thanks  for the explaination.
May be recovery mechanism  implementation could be considered.


>
> Regards,
> Iztok Jeras
>
>