Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 1 Apr 2022 09:04:17 -0500
From:   Rob Herring <robh@kernel.org>
To:     niravkumar.l.rabara@intel.com
Cc:     Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Derek Kiernan <derek.kiernan@xilinx.com>,
        Dragan Cvetic <dragan.cvetic@xilinx.com>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Add Altera hardware mutex driver
Message-ID: <YkcGYcGVkai3eu1E@robh.at.kernel.org>
References: <20220331214911.27194-1-niravkumar.l.rabara@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220331214911.27194-1-niravkumar.l.rabara@intel.com>
Precedence: bulk

On Fri, Apr 01, 2022 at 05:49:11AM +0800, niravkumar.l.rabara@intel.com wrote:
> From: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> 
> Altera hardware mutex soft IP provides hardware assistance for
> synchronization and mutual exclusion between processors in
> asymmetric/symmetric multiprocessing (AMP/SMP) system or
> multi processes/threads in uniprocessor system.
> 
> Signed-off-by: Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> ---
>  .../bindings/misc/altera-hwmutex.yaml         |  47 +++

Bindings should be separate patch. We have a subsystem/class for this 
type of h/w. The binding (and driver) belongs there.

>  drivers/misc/Kconfig                          |   6 +
>  drivers/misc/Makefile                         |   1 +
>  drivers/misc/altera_hwmutex.c                 | 321 ++++++++++++++++++
>  include/linux/altera_hwmutex.h                |  42 +++
>  5 files changed, 417 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/altera-hwmutex.yaml
>  create mode 100644 drivers/misc/altera_hwmutex.c
>  create mode 100644 include/linux/altera_hwmutex.h
> 
> diff --git a/Documentation/devicetree/bindings/misc/altera-hwmutex.yaml b/Documentation/devicetree/bindings/misc/altera-hwmutex.yaml
> new file mode 100644
> index 000000000000..57a9ea19c563
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/altera-hwmutex.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/misc/altera-hwmutex.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Altera hardware mutex
> +
> +maintainers:
> +  - Niravkumar L Rabara <niravkumar.l.rabara@intel.com>
> +
> +description:
> +  Altera hardware mutex can provide hardware assistance for synchronization
> +  and mutual exclusion between processors in asymmetric/symmetric multiprocessing
> +  (AMP/SMP) system or multi processes/threads in uniprocessor system.

Link to IP documentation?

> +
> +properties:
> +  compatible:
> +    enum:
> +      - altr,hwmutex-1.0

1.0? I feel like you made up this version.

> +      - client-1.0

No.

> +
> +  reg:
> +    items:
> +      - description: physical address of hw mutex and length of memory mapped
> +         region
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +examples:
> +  - |
> +    mutex0: mutex0@100 {
> +        compatible = "altr,hwmutex-1.0";
> +        reg = <0x100 0x8>;
> +    };
> +
> +
> +   #Example of mutex's client node that includes mutex phandle    
> +   #mclient0: mclient0@200 {
> +   #     compatible = "client-1.0";
> +   # 	reg = <0x200 0x10>;
> +   #	mutex = <&mutex0>;

We have a standard binding for this.

> +   # };
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 0f5a49fc7c9e..707acf740c6f 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -435,6 +435,12 @@ config DW_XDATA_PCIE
>  
>  	  If unsure, say N.
>  
> +config ALTERA_HWMUTEX
> +       tristate "Altera Hardware Mutex"
> +       help
> +         This option enables device driver support for Altera Hardware Mutex.
> +         Say Y here if you want to use the Altera hardware mutex support.
> +
>  config PCI_ENDPOINT_TEST
>  	depends on PCI
>  	select CRC32
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index a086197af544..6fcbbd36b3cf 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_PCH_PHUB)		+= pch_phub.o
>  obj-y				+= ti-st/
>  obj-y				+= lis3lv02d/
>  obj-$(CONFIG_ALTERA_STAPL)	+=altera-stapl/
> +obj-$(CONFIG_ALTERA_HWMUTEX)   += altera_hwmutex.o
>  obj-$(CONFIG_INTEL_MEI)		+= mei/
>  obj-$(CONFIG_VMWARE_VMCI)	+= vmw_vmci/
>  obj-$(CONFIG_LATTICE_ECP3_CONFIG)	+= lattice-ecp3-config.o
> diff --git a/drivers/misc/altera_hwmutex.c b/drivers/misc/altera_hwmutex.c
> new file mode 100644
> index 000000000000..45f98e4b13d0
> --- /dev/null
> +++ b/drivers/misc/altera_hwmutex.c
> @@ -0,0 +1,321 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright Intel Corporation (C) 2022. All rights reserved
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.

Drop the license text. You only need SPDX-License-Identifier