Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 19 Jan 2022 16:30:12 +0100
From:   Jean Delvare <jdelvare@suse.de>
To:     Terry Bowman <terry.bowman@amd.com>
Cc:     <linux@roeck-us.net>, <linux-watchdog@vger.kernel.org>,
        <jdelvare@suse.com>, <linux-i2c@vger.kernel.org>, <wsa@kernel.org>,
        <andy.shevchenko@gmail.com>, <rafael.j.wysocki@intel.com>,
        <linux-kernel@vger.kernel.org>, <wim@linux-watchdog.org>,
        <rrichter@amd.com>, <thomas.lendacky@amd.com>,
        <Nehal-bakulchandra.Shah@amd.com>, <Basavaraj.Natikar@amd.com>,
        <Shyam-sundar.S-k@amd.com>, <Mario.Limonciello@amd.com>
Subject: Re: [PATCH v3 0/4] Watchdog: sp5100_tco: Replace cd6h/cd7h port I/O
 accesses with MMIO accesses
Message-ID: <20220119163012.4274665d@endymion>
In-Reply-To: <20220118202234.410555-1-terry.bowman@amd.com>
References: <20220118202234.410555-1-terry.bowman@amd.com>
Organization: SUSE Linux
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Terry,

On Tue, 18 Jan 2022 14:22:30 -0600, Terry Bowman wrote:
> This series uses request_mem_region() to synchronize accesses to the MMIO
> registers mentioned above. request_mem_region() is missing the retry
> logic in the case the resource is busy. As a result, request_mem_region()
> will fail immediately if the resource is busy. The 'muxed' variant is
> needed here but request_muxed_mem_region() is not defined to use.  I will
> follow up with another patch series to define the
> request_muxed_mem_region() and use in both drivers.

Shouldn't this be done the other way around, first introducing
request_muxed_mem_region() and then using it directly in both drivers,
rather than having a temporary situation where a failure can happen?

As far as I'm concerned, the patch series you just posted are
acceptable only if request_muxed_mem_region() gets accepted too.
Otherwise we end up with the situation where a driver could randomly
fail.

-- 
Jean Delvare
SUSE L3 Support