linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robert Richter <rrichter@amd.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Terry Bowman <Terry.Bowman@amd.com>,
	linux-watchdog@vger.kernel.org, wim@linux-watchdog.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Watchdog: sp5100_tco: Replace watchdog cd6h/cd7h port I/O accesses with MMIO accesses
Date: Mon, 6 Sep 2021 14:05:34 +0200	[thread overview]
Message-ID: <YTYEDvTTrE5JXlk4@rric.localdomain> (raw)
In-Reply-To: <416a67a7-646b-eb8d-b617-80cbbbc028c6@roeck-us.net>

Guenter,

On 13.08.21 15:37:30, Guenter Roeck wrote:
> I am sorry, I don't understand why the new code can not use devm functions,

devm was not chosen here as the iomem is only used during
initialization. The resource is released at the end of it. Using devm
would keep it open and carry it around including its data until the
device is released which we wanted to avoid. This would also add
unnecessary data to struct sp5100_tco and/or the device's data.

> why the new data structure is necessary in the first place,

Instead we used struct efch_cfg to carry all that temporary data
required during initialization. This is destroyed at the end of
sp5100_tco_setupdevice().

> and why it is
> not possible to improve alignment with the existing code.

The driver serves 3 to 4 platforms now. To refactor it a core driver
is needed plus sub-drivers for each platform. We decided not to change
existing code here for older platforms and keep it as it is for them.

Moreover, the efch driver only needs changes to its initialization
code (using mmio instead of ioports), everything else remains the
same.

> This will require
> a substantial amount of time to review to ensure that the changes are not
> excessive (at first glance it for sure looks like that to me).

This patch implements mostly helper functions and adds alternative
code paths for the mmio case. For an easier review we could split the
patch in about 3-4 patches, would that help?

Thanks,

-Robert

      parent reply	other threads:[~2021-09-06 12:05 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-13 21:32 [PATCH] Watchdog: sp5100_tco: Replace watchdog cd6h/cd7h port I/O accesses with MMIO accesses Terry Bowman
2021-08-13 22:37 ` Guenter Roeck
2021-08-16 21:29   ` Terry Bowman
2021-08-18 20:34     ` Guenter Roeck
2021-08-23 17:58       ` Terry Bowman
2021-09-06 12:05   ` Robert Richter [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YTYEDvTTrE5JXlk4@rric.localdomain \
    --to=rrichter@amd.com \
    --cc=Terry.Bowman@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=wim@linux-watchdog.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).