On Fri, May 08, 2015 at 12:40:49AM +0800, Fu Wei wrote:
> Hi Guenter,
>
>
>
> On 5 May 2015 at 00:36, Guenter Roeck <linux@roeck-us.net> wrote:
>
> > On Tue, May 05, 2015 at 12:03:05AM +0800, Fu Wei wrote:
> > > Hi Guenter,
> > >
> > > Great thanks for your feedback, will fix them ASAP.
> > > Some feedback and comment inline below :
> > >
> > [ ... ]
> >
> > > >> +
> > > >> +static int sbsa_gwdt_stop(struct watchdog_device *wdd)
> > > >> +{
> > > >> +       struct sbsa_gwdt *gwdt = to_sbsa_gwdt(wdd);
> > > >> +       struct sbsa_gwdt_device_data *device_data =
> > &gwdt->device_data;
> > > >> +
> > > >> +       u32 status;
> > > >> +
> > > >> +       mutex_lock(&device_data->lock);
> > > >> +       status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);
> > > >> +       status &= ~SBSA_GWDT_WCS_EN;
> > > >> +       /* writing WCS will cause an explicit watchdog refresh */
> > > >> +       sbsa_gwdt_cf_write(SBSA_GWDT_WCS, status, wdd);
> > > >> +       status = sbsa_gwdt_cf_read(SBSA_GWDT_WCS, wdd);
> > > >> +       mutex_unlock(&device_data->lock);
> > > >> +
> > > >> +       /* Check if the watchdog was disabled */
> > > >> +       if (status & SBSA_GWDT_WCS_EN)
> > > >> +               return -EACCES;
> > > >>
> > > >
> > > > Same as above.
> > >
> > >
> > > For these two, maybe EPERM?
> > >
> > "Operation not permitted" seems just as wrong. Both usually reflect a
> > software
> > state, not a failure to write to hardware.
> >
> > The question here really is if this can really happen, and, if it does,
> > why it happens. In general it is quite uncommon for watchdog drivers to
> > verify if such writes were successful.
> >
>
> I think , EIO is the best one,  we config the reg to enable it , but fail,
> the mean we have problem on bus/IO,
> Do you agree?
>

Ok. Question though is still if this is a realistic error.
In other words, is there any reason to believe that this can happen ?
Because it is not really common in the kernel to read back a value from an
io register unless there is reason to believe that a write can fail.
Or, in other words, io writes are commonly seen to be reliable unless
there is reason to believe otherwise.

Thanks,
Guenter