From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S932849AbdKBRtZ (ORCPT <rfc822;w@1wt.eu>);
        Thu, 2 Nov 2017 13:49:25 -0400
Received: from mail-pg0-f65.google.com ([74.125.83.65]:43610 "EHLO
        mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1755528AbdKBRtX (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 2 Nov 2017 13:49:23 -0400
X-Google-Smtp-Source: ABhQp+SSnpfUOYgsIWNln619NX850LL58niGBOaBR9lJBK3ki/jkp4O3mig9dAR2y8LDaOCu4g/t3A==
Subject: Re: [PATCH 3/7] MIPS: Octeon: Add a global resource manager.
To: David Daney <ddaney@caviumnetworks.com>, Andrew Lunn <andrew@lunn.ch>,
        David Daney <david.daney@cavium.com>
Cc: linux-mips@linux-mips.org, ralf@linux-mips.org,
        James Hogan <james.hogan@mips.com>, netdev@vger.kernel.org,
        "David S. Miller" <davem@davemloft.net>,
        Rob Herring <robh+dt@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
        linux-kernel@vger.kernel.org,
        "Steven J. Hill" <steven.hill@cavium.com>, devicetree@vger.kernel.org,
        Carlos Munoz <cmunoz@cavium.com>
References: <20171102003606.19913-1-david.daney@cavium.com>
 <20171102003606.19913-4-david.daney@cavium.com>
 <20171102122327.GE4772@lunn.ch>
 <caeae680-915a-d08c-d220-899db0970328@caviumnetworks.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Message-ID: <6ff04462-88ec-a972-4084-e26ea3cb630a@gmail.com>
Date: Thu, 2 Nov 2017 10:49:19 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <caeae680-915a-d08c-d220-899db0970328@caviumnetworks.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 11/02/2017 09:03 AM, David Daney wrote:
> On 11/02/2017 05:23 AM, Andrew Lunn wrote:
>>> +static void res_mgr_lock(void)
>>> +{
>>> +    unsigned int tmp;
>>> +    u64 lock = (u64)&res_mgr_info->rlock;
>>> +
>>> +    __asm__ __volatile__(
>>> +        ".set noreorder\n"
>>> +        "1: ll   %[tmp], 0(%[addr])\n"
>>> +        "   bnez %[tmp], 1b\n"
>>> +        "   li   %[tmp], 1\n"
>>> +        "   sc   %[tmp], 0(%[addr])\n"
>>> +        "   beqz %[tmp], 1b\n"
>>> +        "   nop\n"
>>> +        ".set reorder\n" :
>>> +        [tmp] "=&r"(tmp) :
>>> +        [addr] "r"(lock) :
>>> +        "memory");
>>> +}
>>> +
>>> +static void res_mgr_unlock(void)
>>> +{
>>> +    u64 lock = (u64)&res_mgr_info->rlock;
>>> +
>>> +    /* Wait until all resource operations finish before unlocking. */
>>> +    mb();
>>> +    __asm__ __volatile__(
>>> +        "sw $0, 0(%[addr])\n" : :
>>> +        [addr] "r"(lock) :
>>> +        "memory");
>>> +
>>> +    /* Force a write buffer flush. */
>>> +    mb();
>>> +}
>>
>> It would be good to add some justification for using your own locks,
>> rather than standard linux locks.
> 
> Yes, I will add that.
> 
> 
>>
>> Is there anything specific to your hardware in this resource manager?
>> I'm just wondering if this should be generic, put somewhere in lib. Or
>> maybe there is already something generic, and you should be using it,
>> not re-inventing the wheel again.
> 
> The systems built around this hardware may have other software running
> on CPUs that are not running the Linux kernel.  The data structures used
> to arbitrate usage of shared system hardware resources use exactly these
> locking primitives, so they cannot be changed to use the Linux locking
> implementation de jour.

Would hwspinlock be a possible option so this is abstracted on the Linux
side using these locking primitives through the hwspinlock layer which
in turn does exactly what is above?
-- 
Florian