* Re: Re: [RESEND PATCH v5 5/5] Documentation/vmcoreinfo: Add documentation for 'TCR_EL1.T1SZ'
From: Scott Branden @ 2020-06-03 18:47 UTC (permalink / raw)
To: Bhupesh Sharma, James Morse, linux-kernel
Cc: Mark Rutland, Ard Biesheuvel, linux-doc, Catalin Marinas, x86,
kexec, linuxppc-dev, Kazuhito Hagio, Dave Anderson, bhupesh.linux,
Will Deacon, linux-arm-kernel, Steve Capper
In-Reply-To: <b7d8d603-d9fe-3e18-c754-baf2015acd16@redhat.com>
Hi Bhupesh,
Would be great to get this patch series upstreamed?
On 2019-12-25 10:49 a.m., Bhupesh Sharma wrote:
> Hi James,
>
> On 12/12/2019 04:02 PM, James Morse wrote:
>> Hi Bhupesh,
>
> I am sorry this review mail skipped my attention due to holidays and
> focus on other urgent issues.
>
>> On 29/11/2019 19:59, Bhupesh Sharma wrote:
>>> Add documentation for TCR_EL1.T1SZ variable being added to
>>> vmcoreinfo.
>>>
>>> It indicates the size offset of the memory region addressed by
>>> TTBR1_EL1
>>
>>> and hence can be used for determining the vabits_actual value.
>>
>> used for determining random-internal-kernel-variable, that might not
>> exist tomorrow.
>>
>> Could you describe how this is useful/necessary if a debugger wants
>> to walk the page
>> tables from the core file? I think this is a better argument.
>>
>> Wouldn't the documentation be better as part of the patch that adds
>> the export?
>> (... unless these have to go via different trees? ..)
>
> Ok, will fix the same in v6 version.
>
>>> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst
>>> b/Documentation/admin-guide/kdump/vmcoreinfo.rst
>>> index 447b64314f56..f9349f9d3345 100644
>>> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
>>> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
>>> @@ -398,6 +398,12 @@ KERNELOFFSET
>>> The kernel randomization offset. Used to compute the page offset. If
>>> KASLR is disabled, this value is zero.
>>> +TCR_EL1.T1SZ
>>> +------------
>>> +
>>> +Indicates the size offset of the memory region addressed by TTBR1_EL1
>>
>>> +and hence can be used for determining the vabits_actual value.
>>
>> 'vabits_actual' may not exist when the next person comes to read this
>> documentation (its
>> going to rot really quickly).
>>
>> I think the first half of this text is enough to say what this is
>> for. You should include
>> words to the effect that its the hardware value that goes with
>> swapper_pg_dir. You may
>> want to point readers to the arm-arm for more details on what the
>> value means.
>
> Ok, got it. Fixed this in v6, which should be on its way shortly.
I can't seem to find v6?
>
> Thanks,
> Bhupesh
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* Re: [RESEND PATCH v9 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
From: Vaibhav Jain @ 2020-06-03 18:11 UTC (permalink / raw)
To: Ira Weiny
Cc: Santosh Sivaraj, linux-nvdimm, linux-kernel, Steven Rostedt,
Oliver O'Halloran, Aneesh Kumar K . V, Dan Williams,
linuxppc-dev
In-Reply-To: <20200602205148.GF1505637@iweiny-DESK2.sc.intel.com>
Hi Ira,
Thanks for reviewing this patch. My responses below:
Ira Weiny <ira.weiny@intel.com> writes:
> On Tue, Jun 02, 2020 at 03:44:37PM +0530, Vaibhav Jain wrote:
>> Introduce support for PAPR NVDIMM Specific Methods (PDSM) in papr_scm
>> module and add the command family NVDIMM_FAMILY_PAPR to the white list
>> of NVDIMM command sets. Also advertise support for ND_CMD_CALL for the
>> nvdimm command mask and implement necessary scaffolding in the module
>> to handle ND_CMD_CALL ioctl and PDSM requests that we receive.
>>
>> The layout of the PDSM request as we expect from libnvdimm/libndctl is
>> described in newly introduced uapi header 'papr_pdsm.h' which
>> defines a new 'struct nd_pdsm_cmd_pkg' header. This header is used
>> to communicate the PDSM request via member
>> 'nd_cmd_pkg.nd_command' and size of payload that need to be
>> sent/received for servicing the PDSM.
>>
>> A new function is_cmd_valid() is implemented that reads the args to
>> papr_scm_ndctl() and performs sanity tests on them. A new function
>> papr_scm_service_pdsm() is introduced and is called from
>> papr_scm_ndctl() in case of a PDSM request is received via ND_CMD_CALL
>> command from libnvdimm.
>>
>> Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Changelog:
>>
>> Resend:
>> * Added ack from Aneesh.
>>
>> v8..v9:
>> * Reduced the usage of term SCM replacing it with appropriate
>> replacement [ Dan Williams, Aneesh ]
>> * Renamed 'papr_scm_pdsm.h' to 'papr_pdsm.h'
>> * s/PAPR_SCM_PDSM_*/PAPR_PDSM_*/g
>> * s/NVDIMM_FAMILY_PAPR_SCM/NVDIMM_FAMILY_PAPR/g
>> * Minor updates to 'papr_psdm.h' to replace usage of term 'SCM'.
>> * Minor update to patch description.
>>
>> v7..v8:
>> * Removed the 'payload_offset' field from 'struct
>> nd_pdsm_cmd_pkg'. Instead command payload is always assumed to start
>> at 'nd_pdsm_cmd_pkg.payload'. [ Aneesh ]
>> * To enable introducing new fields to 'struct nd_pdsm_cmd_pkg',
>> 'reserved' field of 10-bytes is introduced. [ Aneesh ]
>> * Fixed a typo in "Backward Compatibility" section of papr_scm_pdsm.h
>> [ Ira ]
>>
>> Resend:
>> * None
>>
>> v6..v7 :
>> * Removed the re-definitions of __packed macro from papr_scm_pdsm.h
>> [Mpe].
>> * Removed the usage of __KERNEL__ macros in papr_scm_pdsm.h [Mpe].
>> * Removed macros that were unused in papr_scm.c from papr_scm_pdsm.h
>> [Mpe].
>> * Made functions defined in papr_scm_pdsm.h as static inline. [Mpe]
>>
>> v5..v6 :
>> * Changed the usage of the term DSM to PDSM to distinguish it from the
>> ACPI term [ Dan Williams ]
>> * Renamed papr_scm_dsm.h to papr_scm_pdsm.h and updated various struct
>> to reflect the new terminology.
>> * Updated the patch description and title to reflect the new terminology.
>> * Squashed patch to introduce new command family in 'ndctl.h' with
>> this patch [ Dan Williams ]
>> * Updated the papr_scm_pdsm method starting index from 0x10000 to 0x0
>> [ Dan Williams ]
>> * Removed redundant license text from the papr_scm_psdm.h file.
>> [ Dan Williams ]
>> * s/envelop/envelope/ at various places [ Dan Williams ]
>> * Added '__packed' attribute to command package header to gaurd
>> against different compiler adding paddings between the fields.
>> [ Dan Williams]
>> * Converted various pr_debug to dev_debug [ Dan Williams ]
>>
>> v4..v5 :
>> * None
>>
>> v3..v4 :
>> * None
>>
>> v2..v3 :
>> * Updated the patch prefix to 'ndctl/uapi' [Aneesh]
>>
>> v1..v2 :
>> * None
>> ---
>> arch/powerpc/include/uapi/asm/papr_pdsm.h | 136 ++++++++++++++++++++++
>> arch/powerpc/platforms/pseries/papr_scm.c | 101 +++++++++++++++-
>> include/uapi/linux/ndctl.h | 1 +
>> 3 files changed, 232 insertions(+), 6 deletions(-)
>> create mode 100644 arch/powerpc/include/uapi/asm/papr_pdsm.h
>>
>> diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
>> new file mode 100644
>> index 000000000000..6407fefcc007
>> --- /dev/null
>> +++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
>> @@ -0,0 +1,136 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>> +/*
>> + * PAPR nvDimm Specific Methods (PDSM) and structs for libndctl
>> + *
>> + * (C) Copyright IBM 2020
>> + *
>> + * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
>> + */
>> +
>> +#ifndef _UAPI_ASM_POWERPC_PAPR_PDSM_H_
>> +#define _UAPI_ASM_POWERPC_PAPR_PDSM_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/*
>> + * PDSM Envelope:
>> + *
>> + * The ioctl ND_CMD_CALL transfers data between user-space and kernel via
>> + * envelope which consists of a header and user-defined payload sections.
>> + * The header is described by 'struct nd_pdsm_cmd_pkg' which expects a
>> + * payload following it and accessible via 'nd_pdsm_cmd_pkg.payload' field.
>> + * There is reserved field that can used to introduce new fields to the
>> + * structure in future. It also tries to ensure that 'nd_pdsm_cmd_pkg.payload'
>> + * lies at a 8-byte boundary.
>> + *
>> + * +-------------+---------------------+---------------------------+
>> + * | 64-Bytes | 16-Bytes | Max 176-Bytes |
>> + * +-------------+---------------------+---------------------------+
>> + * | nd_pdsm_cmd_pkg | |
>> + * |-------------+ | |
>> + * | nd_cmd_pkg | | |
>> + * +-------------+---------------------+---------------------------+
>> + * | nd_family | | |
>> + * | nd_size_out | cmd_status | |
>> + * | nd_size_in | payload_version | payload |
>> + * | nd_command | reserved | |
>> + * | nd_fw_size | | |
>> + * +-------------+---------------------+---------------------------+
>> + *
>> + * PDSM Header:
>> + *
>> + * The header is defined as 'struct nd_pdsm_cmd_pkg' which embeds a
>> + * 'struct nd_cmd_pkg' instance. The PDSM command is assigned to member
>> + * 'nd_cmd_pkg.nd_command'. Apart from size information of the envelope which is
>> + * contained in 'struct nd_cmd_pkg', the header also has members following
> ^^^^^
> ... the ...
Thanks for catching this will get it fixed.
>
>> + * members:
>> + *
>> + * 'cmd_status' : (Out) Errors if any encountered while servicing PDSM.
>> + * 'payload_version' : (In/Out) Version number associated with the payload.
>> + * 'reserved' : Not used and reserved for future.
>> + *
>> + * PDSM Payload:
>> + *
>> + * The layout of the PDSM Payload is defined by various structs shared between
>> + * papr_scm and libndctl so that contents of payload can be interpreted. During
>> + * servicing of a PDSM the papr_scm module will read input args from the payload
>> + * field by casting its contents to an appropriate struct pointer based on the
>> + * PDSM command. Similarly the output of servicing the PDSM command will be
>> + * copied to the payload field using the same struct.
>> + *
>> + * 'libnvdimm' enforces a hard limit of 256 bytes on the envelope size, which
>> + * leaves around 176 bytes for the envelope payload (ignoring any padding that
>> + * the compiler may silently introduce).
>> + *
>> + * Payload Version:
>> + *
>> + * A 'payload_version' field is present in PDSM header that indicates a specific
>> + * version of the structure present in PDSM Payload for a given PDSM command.
>> + * This provides backward compatibility in case the PDSM Payload structure
>> + * evolves and different structures are supported by 'papr_scm' and 'libndctl'.
>> + *
>> + * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version
>> + * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
>> + * module when servicing the PDSM envelope checks the 'payload_version' and then
>> + * uses 'payload struct version' == MIN('payload_version field',
>> + * 'max payload-struct-version supported by papr_scm') to service the PDSM.
>> + * After servicing the PDSM, 'papr_scm' put the negotiated version of payload
>> + * struct in returned 'payload_version' field.
>> + *
>> + * Libndctl on receiving the envelope back from papr_scm again checks the
>> + * 'payload_version' field and based on it use the appropriate version dsm
>> + * struct to parse the results.
>> + *
>> + * Backward Compatibility:
>> + *
>> + * Above scheme of exchanging different versioned PDSM struct between libndctl
>> + * and papr_scm should provide backward compatibility until following two
>> + * assumptions/conditions when defining new PDSM structs hold:
>> + *
>> + * Let T(X) = { set of attributes in PDSM struct 'T' versioned X }
>> + *
>> + * 1. T(X) is a proper subset of T(Y) if Y > X.
>> + * i.e Each new version of PDSM struct should retain existing struct
>> + * attributes from previous version
>> + *
>> + * 2. If an entity (libndctl or papr_scm) supports a PDSM struct T(X) then
>> + * it should also support T(1), T(2)...T(X - 1).
>> + * i.e When adding support for new version of a PDSM struct, libndctl
>> + * and papr_scm should retain support of the existing PDSM struct
>> + * version they support.
>
> Please see this thread for an example why versions are a bad idea in UAPIs:
>
> https://lkml.org/lkml/2020/3/26/213
>
> While the use of version is different in that thread the fundamental issues are
> the same. You end up with some weird matrix of supported features and
> structure definitions. For example, you are opening up the possibility of
> changing structures with a different version for no good reason.
Not really sure I understand the statement correctly "you are opening up
the possibility of changing structures with a different version for no
good reason."
We want to return more data in the struct in future iterations. So
'changing structure with different version' is something we are
expecting.
With the backward compatibility constraints 1 & 2 above, it will ensure
that support matrix looks like a lower traingular matrix with each
successive version supporting previous version attributes. So supporting
future versions is relatively simplified.
>
> Also having the user query with version Z and get back version X (older) is
> odd. Generally if the kernel does not know about a feature (ie version Z of
> the structure) it should return -EINVAL and let the user figure out what to do.
> The user may just give up or they could try a different query.
>
Considering the flow of ndctl/libndctl this is needed. libndctl will
usually issues only one CMD_CALL ioctl to kernel and if that fails then
an error is reported and ndctl will exit loosing state.
Adding mechanism in libndctl to reissue CMD_CALL ioctl to fetch a
appropriate version of pdsm struct is going to be considerably more
work.
This version fall-back mechanism, ensures that libndctl will receive
usable data without having to reissue a more CMD_CALL ioctls.
>> + */
>> +
>> +/* PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
>> +struct nd_pdsm_cmd_pkg {
>> + struct nd_cmd_pkg hdr; /* Package header containing sub-cmd */
>> + __s32 cmd_status; /* Out: Sub-cmd status returned back */
>> + __u16 reserved[5]; /* Ignored and to be used in future */
>
> How do you know when reserved is used for something else in the future? Is
> reserved guaranteed (and checked by the code) to be 0?
For current set of pdsm requests ignore these reserved fields. However a
future pdsm request can leverage these reserved fields. So papr_scm
just bind the usage of these fields with the value of
'nd_cmd_pkg.nd_command' that indicates the pdsm request.
That being said checking if the reserved fields are set to 0 will be a
good measure. Will add this check in next iteration.
>
>> + __u16 payload_version; /* In/Out: version of the payload */
>
> Why is payload_version after reserved?
Want to place the payload version field just before the payload data so
that it can be accessed with simple pointer arithmetic.
>
>> + __u8 payload[]; /* In/Out: Sub-cmd data buffer */
>> +} __packed;
>> +
>> +/*
>> + * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
>> + * via 'nd_pdsm_cmd_pkg.hdr.nd_command' member of the ioctl struct
>> + */
>> +enum papr_pdsm {
>> + PAPR_PDSM_MIN = 0x0,
>> + PAPR_PDSM_MAX,
>> +};
>> +
>> +/* Convert a libnvdimm nd_cmd_pkg to pdsm specific pkg */
>> +static inline struct nd_pdsm_cmd_pkg *nd_to_pdsm_cmd_pkg(struct nd_cmd_pkg *cmd)
>> +{
>> + return (struct nd_pdsm_cmd_pkg *) cmd;
>> +}
>> +
>> +/* Return the payload pointer for a given pcmd */
>> +static inline void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
>> +{
>> + if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
>> + return NULL;
>> + else
>> + return (void *)(pcmd->payload);
>> +}
>> +
>> +#endif /* _UAPI_ASM_POWERPC_PAPR_PDSM_H_ */
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 149431594839..5e2237e7ec08 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -15,13 +15,15 @@
>> #include <linux/seq_buf.h>
>>
>> #include <asm/plpar_wrappers.h>
>> +#include <asm/papr_pdsm.h>
>>
>> #define BIND_ANY_ADDR (~0ul)
>>
>> #define PAPR_SCM_DIMM_CMD_MASK \
>> ((1ul << ND_CMD_GET_CONFIG_SIZE) | \
>> (1ul << ND_CMD_GET_CONFIG_DATA) | \
>> - (1ul << ND_CMD_SET_CONFIG_DATA))
>> + (1ul << ND_CMD_SET_CONFIG_DATA) | \
>> + (1ul << ND_CMD_CALL))
>>
>> /* DIMM health bitmap bitmap indicators */
>> /* SCM device is unable to persist memory contents */
>> @@ -350,16 +352,97 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,
>> return 0;
>> }
>>
>> +/*
>> + * Validate the inputs args to dimm-control function and return '0' if valid.
>> + * This also does initial sanity validation to ND_CMD_CALL sub-command packages.
>> + */
>> +static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
>> + unsigned int buf_len)
>> +{
>> + unsigned long cmd_mask = PAPR_SCM_DIMM_CMD_MASK;
>> + struct nd_pdsm_cmd_pkg *pkg = nd_to_pdsm_cmd_pkg(buf);
>> + struct papr_scm_priv *p;
>> +
>> + /* Only dimm-specific calls are supported atm */
>> + if (!nvdimm)
>> + return -EINVAL;
>> +
>> + /* get the provider date from struct nvdimm */
>
> s/date/data
Thanks for point this out. Will fix this in next iteration.
>
>> + p = nvdimm_provider_data(nvdimm);
>> +
>> + if (!test_bit(cmd, &cmd_mask)) {
>> + dev_dbg(&p->pdev->dev, "Unsupported cmd=%u\n", cmd);
>> + return -EINVAL;
>> + } else if (cmd == ND_CMD_CALL) {
>> +
>> + /* Verify the envelope package */
>> + if (!buf || buf_len < sizeof(struct nd_pdsm_cmd_pkg)) {
>> + dev_dbg(&p->pdev->dev, "Invalid pkg size=%u\n",
>> + buf_len);
>> + return -EINVAL;
>> + }
>> +
>> + /* Verify that the PDSM family is valid */
>> + if (pkg->hdr.nd_family != NVDIMM_FAMILY_PAPR) {
>> + dev_dbg(&p->pdev->dev, "Invalid pkg family=0x%llx\n",
>> + pkg->hdr.nd_family);
>> + return -EINVAL;
>> +
>> + }
>> +
>> + /* We except a payload with all PDSM commands */
>> + if (pdsm_cmd_to_payload(pkg) == NULL) {
>> + dev_dbg(&p->pdev->dev,
>> + "Empty payload for sub-command=0x%llx\n",
>> + pkg->hdr.nd_command);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + /* Command looks valid */
>
> I assume the first command to be implemented also checks the { nd_command,
> payload_version, payload length } for correctness?
Yes the pdsm service functions do check the payload_version and
payload_length. Please see the papr_pdsm_health() that services the
PAPR_PDSM_HEALTH pdsm in Patch-5
>
>> + return 0;
>> +}
>> +
>> +static int papr_scm_service_pdsm(struct papr_scm_priv *p,
>> + struct nd_pdsm_cmd_pkg *call_pkg)
>> +{
>> + /* unknown subcommands return error in packages */
>> + if (call_pkg->hdr.nd_command <= PAPR_PDSM_MIN ||
>> + call_pkg->hdr.nd_command >= PAPR_PDSM_MAX) {
>> + dev_dbg(&p->pdev->dev, "Invalid PDSM request 0x%llx\n",
>> + call_pkg->hdr.nd_command);
>> + call_pkg->cmd_status = -EINVAL;
>> + return 0;
>> + }
>> +
>> + /* Depending on the DSM command call appropriate service routine */
>> + switch (call_pkg->hdr.nd_command) {
>> + default:
>> + dev_dbg(&p->pdev->dev, "Unsupported PDSM request 0x%llx\n",
>> + call_pkg->hdr.nd_command);
>> + call_pkg->cmd_status = -ENOENT;
>> + return 0;
>> + }
>> +}
>> +
>> static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
>> struct nvdimm *nvdimm, unsigned int cmd, void *buf,
>> unsigned int buf_len, int *cmd_rc)
>> {
>> struct nd_cmd_get_config_size *get_size_hdr;
>> struct papr_scm_priv *p;
>> + struct nd_pdsm_cmd_pkg *call_pkg = NULL;
>> + int rc;
>>
>> - /* Only dimm-specific calls are supported atm */
>> - if (!nvdimm)
>> - return -EINVAL;
>> + /* Use a local variable in case cmd_rc pointer is NULL */
>> + if (cmd_rc == NULL)
>> + cmd_rc = &rc;
>
> Why is this needed? AFAICT The caller of papr_scm_ndctl does not specify null
> and you did not change it.
This pointer is coming from outside the papr_scm code hence need to be
defensive here. Also as per[1] cmd_rc is "translation of firmware status"
and not every caller would need it hence making this pointer optional.
This is evident in acpi_nfit_blk_get_flags() where the 'nd_desc->ndctl'
is called with 'cmd_rc == NULL'.
[1] https://lore.kernel.org/linux-nvdimm/CAPcyv4hE_FG0YZXJVA1G=CBq8b9e0K54jxk5Sq5UKU-dnWT2Kg@mail.gmail.com/
>
>> +
>> + *cmd_rc = is_cmd_valid(nvdimm, cmd, buf, buf_len);
>> + if (*cmd_rc) {
>> + pr_debug("Invalid cmd=0x%x. Err=%d\n", cmd, *cmd_rc);
>> + return *cmd_rc;
>> + }
>>
>> p = nvdimm_provider_data(nvdimm);
>>
>> @@ -381,13 +464,19 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
>> *cmd_rc = papr_scm_meta_set(p, buf);
>> break;
>>
>> + case ND_CMD_CALL:
>> + call_pkg = nd_to_pdsm_cmd_pkg(buf);
>> + *cmd_rc = papr_scm_service_pdsm(p, call_pkg);
>> + break;
>> +
>> default:
>> - return -EINVAL;
>> + dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd);
>> + *cmd_rc = -EINVAL;
>
> Is this change related? If there is a bug where there is a caller of
> papr_scm_ndctl() with cmd_rc == NULL this should be a separate patch to fix
> that issue.
This simplifies a bit debugging of errors reported in
papr_scm_ndctl() as it ensures that subsequest dev_dbg "Returned with
cmd_rc" is always logged.
I think, this is a too small change to be carved out as an independent
patch. Also this doesnt change the behaviour of the code except logging
some more error info.
However, If you feel too strongly about it I will spin a separate patch
in this patch series for this.
>
> Ira
>
>> }
>>
>> dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
>>
>> - return 0;
>> + return *cmd_rc;
>> }
>>
>> static ssize_t flags_show(struct device *dev,
>> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
>> index de5d90212409..0e09dc5cec19 100644
>> --- a/include/uapi/linux/ndctl.h
>> +++ b/include/uapi/linux/ndctl.h
>> @@ -244,6 +244,7 @@ struct nd_cmd_pkg {
>> #define NVDIMM_FAMILY_HPE2 2
>> #define NVDIMM_FAMILY_MSFT 3
>> #define NVDIMM_FAMILY_HYPERV 4
>> +#define NVDIMM_FAMILY_PAPR 5
>>
>> #define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\
>> struct nd_cmd_pkg)
>> --
>> 2.26.2
>>
--
Cheers
~ Vaibhav
^ permalink raw reply
* Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
From: Madhavan Srinivasan @ 2020-06-03 16:34 UTC (permalink / raw)
To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Madhavan Srinivasan
In-Reply-To: <20200602025612.62707-1-aik@ozlabs.ru>
On 6/2/20 8:26 AM, Alexey Kardashevskiy wrote:
> The bhrb_filter_map ("The Branch History Rolling Buffer") callback is
> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
> generic_compat_pmu which does not have this callback and crashed occur.
>
> This add a NULL pointer check for bhrb_filter_map() which behaves as if
> the callback returned an error.
>
> This does not add the same check for config_bhrb() as the only caller
> checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.
Changes looks fine.
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>
The commit be80e758d0c2e ('powerpc/perf: Add generic compat mode pmu
driver')
which introduced generic_compat_pmu was merged in v5.2. So we need to
CC stable starting from 5.2 :( . My bad, sorry.
Maddy
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 3dcfecf858f3..36870569bf9c 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event *event, int ef_flags)
> ret = 0;
> out:
> if (has_branch_stack(event)) {
> - power_pmu_bhrb_enable(event);
> - cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
> - event->attr.branch_sample_type);
> + u64 bhrb_filter = -1;
> +
> + if (ppmu->bhrb_filter_map)
> + bhrb_filter = ppmu->bhrb_filter_map(
> + event->attr.branch_sample_type);
> +
> + if (bhrb_filter != -1) {
> + cpuhw->bhrb_filter = bhrb_filter;
> + power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
> + }
> }
>
> perf_pmu_enable(event->pmu);
> @@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct perf_event *event)
> int n;
> int err;
> struct cpu_hw_events *cpuhw;
> - u64 bhrb_filter;
>
> if (!ppmu)
> return -ENOENT;
> @@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct perf_event *event)
> err = power_check_constraints(cpuhw, events, cflags, n + 1);
>
> if (has_branch_stack(event)) {
> - bhrb_filter = ppmu->bhrb_filter_map(
> + u64 bhrb_filter = -1;
> +
> + if (ppmu->bhrb_filter_map)
> + bhrb_filter = ppmu->bhrb_filter_map(
> event->attr.branch_sample_type);
>
> if (bhrb_filter == -1) {
^ permalink raw reply
* Re: [mainline][Oops][bisected 2ba3e6 ] 5.7.0 boot fails with kernel panic on powerpc
From: Satheesh Rajendran @ 2020-06-03 14:47 UTC (permalink / raw)
To: Joerg Roedel
Cc: sachinp, Stephen Rothwell, linuxppc-dev, linux-kernel,
Steven Rostedt, Abdul Haleem, linux-next, aneesh.kumar, akpm,
manvanth, hch
In-Reply-To: <20200603133257.GL6857@suse.de>
On Wed, Jun 03, 2020 at 03:32:57PM +0200, Joerg Roedel wrote:
> On Wed, Jun 03, 2020 at 04:20:57PM +0530, Abdul Haleem wrote:
> > @Joerg, Could you please have a look?
>
> Can you please try the attached patch?
Hi Joerg,
I did hit the similar boot failue on a Power9 baremetal box(mentioned in Note) and
your below patch helped solving that for my environment and
am able to boot the system fine.
...
Fedora 31 (Thirty One)
Kernel 5.7.0-gd6f9469a0-dirty on an ppc64le (hvc0)
login:
Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Note: for the record, here is the boot failure call trace.
[ 0.023555] mempolicy: Enabling automatic NUMA balancing. Configure with numa_balancing= or the kernel.numa_balancing sysctl
[ 0.023582] pid_max: default: 163840 minimum: 1280
[ 0.035014] BUG: Unable to handle kernel data access on read at 0xc000006000000000
[ 0.035058] Faulting instruction address: 0xc000000000382304
[ 0.035074] Oops: Kernel access of bad area, sig: 11 [#1]
[ 0.035097] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
[ 0.035113] Modules linked in:
[ 0.035136] CPU: 24 PID: 0 Comm: swapper/24 Not tainted 5.7.0-gd6f9469a0 #1
[ 0.035161] NIP: c000000000382304 LR: c00000000038407c CTR: 0000000000000000
[ 0.035197] REGS: c00000000167f930 TRAP: 0300 Not tainted (5.7.0-gd6f9469a0)
[ 0.035241] MSR: 9000000002009033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE> CR: 42022422 XER: 00000000
[ 0.035294] CFAR: c0000000003822fc DAR: c000006000000000 DSISR: 40000000 IRQMASK: 0
[ 0.035294] GPR00: c00000000038407c c00000000167fbc0 c00000000168090[ 150.252645597,5] OPAL: Reboot request...
[ 150.252928266,5] RESET: Initiating fast reboot 1...
0 c008000000000000
[ 0.035294] GPR04: ffffffffffffffff 00000000000001ff c0080000001fffff 0000000000000060
[ 0.035294] GPR08: 0000000060000000 0000000000000005 c000006000000000 c008000000200000
[ 0.035294] GPR12: 0000000022022422 c000000001870000 c000000000000000 c008000000000000
[ 0.035294] GPR16: c008000007ffffff c008000000200000 0000000000000000 c000006000000000
[ 0.035294] GPR20: c008000008000000 c008000008000000 c008000007ffffff c008000007ffffff
[ 0.035294] GPR24: c00000000163f7c8 c00000000172d0c0 0000000000000001 0000000000000001
[ 0.035294] GPR28: c000000001708000 c00000000172d0c8 0000000000000000 c008000008000000
[ 0.035622] NIP [c000000000382304] map_kernel_range_noflush+0x274/0x510
[ 0.035657] LR [c00000000038407c] __vmalloc_node_range+0x2ec/0x3a0
[ 0.035690] Call Trace:
[ 0.035709] [c00000000167fbc0] [c00000000038d848] __alloc_pages_nodemask+0x158/0x3f0 (unreliable)
[ 0.035750] [c00000000167fc90] [c00000000038407c] __vmalloc_node_range+0x2ec/0x3a0
[ 0.035787] [c00000000167fd40] [c000000000384268] __vmalloc+0x58/0x70
[ 0.035823] [c00000000167fdb0] [c000000001056db8] alloc_large_system_hash+0x204/0x304
[ 0.035870] [c00000000167fe60] [c00000000105c1f0] vfs_caches_init+0xd8/0x138
[ 0.035916] [c00000000167fee0] [c0000000010242a0] start_kernel+0x644/0x6ec
[ 0.035960] [c00000000167ff90] [c00000000000ca9c] start_here_common+0x1c/0x400
[ 0.036004] Instruction dump:
[ 0.036016] 3af4ffff 60000000 60000000 38c90010 7f663036 7d667a14 7cc600d0 7d713038
[ 0.036038] 38d1ffff 7c373040 41810008 7e91a378 <e8b30000> 2c250000 418201b4 7f464830
[ 0.036083] ---[ end trace c7e72029dfacc217 ]---
[ 0.036114]
[ 1.036223] Kernel panic - not syncing: Attempted to kill the idle task!
[ 1.036858] Rebooting in 10 seconds..
Regards,
-Satheesh.
>
> diff --git a/include/asm-generic/5level-fixup.h b/include/asm-generic/5level-fixup.h
> index 58046ddc08d0..afbab31fbd7e 100644
> --- a/include/asm-generic/5level-fixup.h
> +++ b/include/asm-generic/5level-fixup.h
> @@ -17,6 +17,11 @@
> ((unlikely(pgd_none(*(p4d))) && __pud_alloc(mm, p4d, address)) ? \
> NULL : pud_offset(p4d, address))
>
> +#define pud_alloc_track(mm, p4d, address, mask) \
> + ((unlikely(pgd_none(*(p4d))) && \
> + (__pud_alloc(mm, p4d, address) || ({*(mask)|=PGTBL_P4D_MODIFIED;0;})))? \
> + NULL : pud_offset(p4d, address))
> +
> #define p4d_alloc(mm, pgd, address) (pgd)
> #define p4d_alloc_track(mm, pgd, address, mask) (pgd)
> #define p4d_offset(pgd, start) (pgd)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7e07f4f490cb..d46bf03b804f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2088,35 +2088,35 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
> NULL : pud_offset(p4d, address);
> }
>
> -static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
> +static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
> unsigned long address,
> pgtbl_mod_mask *mod_mask)
> -
> {
> - if (unlikely(pgd_none(*pgd))) {
> - if (__p4d_alloc(mm, pgd, address))
> + if (unlikely(p4d_none(*p4d))) {
> + if (__pud_alloc(mm, p4d, address))
> return NULL;
> - *mod_mask |= PGTBL_PGD_MODIFIED;
> + *mod_mask |= PGTBL_P4D_MODIFIED;
> }
>
> - return p4d_offset(pgd, address);
> + return pud_offset(p4d, address);
> }
>
> -#endif /* !__ARCH_HAS_5LEVEL_HACK */
> -
> -static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
> +static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
> unsigned long address,
> pgtbl_mod_mask *mod_mask)
> +
> {
> - if (unlikely(p4d_none(*p4d))) {
> - if (__pud_alloc(mm, p4d, address))
> + if (unlikely(pgd_none(*pgd))) {
> + if (__p4d_alloc(mm, pgd, address))
> return NULL;
> - *mod_mask |= PGTBL_P4D_MODIFIED;
> + *mod_mask |= PGTBL_PGD_MODIFIED;
> }
>
> - return pud_offset(p4d, address);
> + return p4d_offset(pgd, address);
> }
>
> +#endif /* !__ARCH_HAS_5LEVEL_HACK */
> +
> static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
> {
> return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?
^ permalink raw reply
* Re: [PATCH 08/10] mtd: rawnand: fsl_upm: Implement exec_op()
From: Miquel Raynal @ 2020-06-03 14:08 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-9-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:20 +0200:
> Implement exec_op() so we can get rid of the legacy interface
> implementation.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 86 ++++++++++++++++++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 9a63e36825d8..03ca20930274 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -194,6 +194,91 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> return ret;
> }
>
> +static int func_exec_instr(struct nand_chip *chip,
> + const struct nand_op_instr *instr)
> +{
> + struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
> + u32 mar, reg_offs = fun->mchip_offsets[fun->mchip_number];
> + unsigned int i;
> + const u8 *out;
> + u8 *in;
> +
> + switch (instr->type) {
> + case NAND_OP_CMD_INSTR:
> + fsl_upm_start_pattern(&fun->upm, fun->upm_cmd_offset);
> + mar = (instr->ctx.cmd.opcode << (32 - fun->upm.width)) |
> + reg_offs;
> + fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
> + fsl_upm_end_pattern(&fun->upm);
> + return 0;
> +
> + case NAND_OP_ADDR_INSTR:
> + fsl_upm_start_pattern(&fun->upm, fun->upm_addr_offset);
> + for (i = 0; i < instr->ctx.addr.naddrs; i++) {
> + mar = (instr->ctx.addr.addrs[i] << (32 - fun->upm.width)) |
> + reg_offs;
> + fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
> + }
> + fsl_upm_end_pattern(&fun->upm);
> + return 0;
> +
> + case NAND_OP_DATA_IN_INSTR:
> + in = instr->ctx.data.buf.in;
> + for (i = 0; i < instr->ctx.data.len; i++)
> + in[i] = in_8(fun->io_base + reg_offs);
> + return 0;
> +
> + case NAND_OP_DATA_OUT_INSTR:
> + out = instr->ctx.data.buf.out;
> + for (i = 0; i < instr->ctx.data.len; i++)
> + out_8(fun->io_base + reg_offs, out[i]);
> + return 0;
> +
> + case NAND_OP_WAITRDY_INSTR:
> + if (!fun->rnb_gpio[fun->mchip_number])
> + return nand_soft_waitrdy(chip, instr->ctx.waitrdy.timeout_ms);
> +
> + return nand_gpio_waitrdy(chip, fun->rnb_gpio[fun->mchip_number],
> + instr->ctx.waitrdy.timeout_ms);
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int fun_exec_op(struct nand_chip *chip, const struct nand_operation *op,
> + bool check_only)
> +{
> + struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
> + unsigned int i;
> + int ret;
> +
> + if (op->cs > NAND_MAX_CHIPS)
> + return -EINVAL;
> +
> + if (check_only)
> + return 0;
> +
> + fun->mchip_number = op->cs;
> +
> + for (i = 0; i < op->ninstrs; i++) {
> + ret = func_exec_instr(chip, &op->instrs[i]);
> + if (ret)
> + return ret;
> +
> + if (op->instrs[i].delay_ns)
> + ndelay(op->instrs[i].delay_ns);
> + }
> +
> + return 0;
> +}
> +
> +static const struct nand_controller_ops fun_ops = {
> + .exec_op = fun_exec_op,
> +};
> +
> static int fun_probe(struct platform_device *ofdev)
> {
> struct fsl_upm_nand *fun;
> @@ -271,6 +356,7 @@ static int fun_probe(struct platform_device *ofdev)
> FSL_UPM_WAIT_WRITE_BYTE;
>
> nand_controller_init(&fun->base);
> + fun->base.ops = &fun_ops;
> fun->dev = &ofdev->dev;
> fun->last_ctrl = NAND_CLE;
>
Looks fine!
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply
* Re: [PATCH 07/10] mtd: rawnand: fsl_upm: Inherit from nand_controller
From: Miquel Raynal @ 2020-06-03 14:01 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-8-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:19 +0200:
> Explicitly inherit from nand_controller instead of relying on the
> nand_chip.legacy.dummy_controller field.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 977b7aad419b..9a63e36825d8 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -24,6 +24,7 @@
> #define FSL_UPM_WAIT_WRITE_BUFFER 0x4
>
> struct fsl_upm_nand {
> + struct nand_controller base;
> struct device *dev;
> struct nand_chip chip;
> int last_ctrl;
> @@ -167,6 +168,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> if (!fun->rnb_gpio[0])
> fun->chip.legacy.dev_ready = fun_chip_ready;
>
> + fun->chip.controller = &fun->base;
> mtd->dev.parent = fun->dev;
>
> flash_np = of_get_next_child(upm_np, NULL);
> @@ -268,6 +270,7 @@ static int fun_probe(struct platform_device *ofdev)
> fun->wait_flags = FSL_UPM_WAIT_RUN_PATTERN |
> FSL_UPM_WAIT_WRITE_BYTE;
>
> + nand_controller_init(&fun->base);
> fun->dev = &ofdev->dev;
> fun->last_ctrl = NAND_CLE;
>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply
* Re: [PATCH 05/10] mtd: rawnand: fsl_upm: Use platform_get_resource() + devm_ioremap_resource()
From: Boris Brezillon @ 2020-06-03 14:00 UTC (permalink / raw)
To: Miquel Raynal
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603155802.12165328@xps13>
On Wed, 3 Jun 2020 15:58:02 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
> 2020 15:49:17 +0200:
>
> > Replace the of_address_to_resource() + devm_ioremap() calls by
> > platform_get_resource() + devm_ioremap_resource() ones which allows us
> > to get rid of one error message since devm_ioremap_resource() already
> > takes care of that.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> > drivers/mtd/nand/raw/fsl_upm.c | 23 +++++++----------------
> > 1 file changed, 7 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> > index a3e3a968891d..54851e9ea784 100644
> > --- a/drivers/mtd/nand/raw/fsl_upm.c
> > +++ b/drivers/mtd/nand/raw/fsl_upm.c
> > @@ -14,7 +14,6 @@
> > #include <linux/mtd/nand_ecc.h>
> > #include <linux/mtd/partitions.h>
> > #include <linux/mtd/mtd.h>
> > -#include <linux/of_address.h>
> > #include <linux/of_platform.h>
> > #include <linux/of_gpio.h>
> > #include <linux/io.h>
> > @@ -197,7 +196,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> > static int fun_probe(struct platform_device *ofdev)
> > {
> > struct fsl_upm_nand *fun;
> > - struct resource io_res;
> > + struct resource *io_res;
> > const __be32 *prop;
> > int rnb_gpio;
> > int ret;
> > @@ -208,13 +207,12 @@ static int fun_probe(struct platform_device *ofdev)
> > if (!fun)
> > return -ENOMEM;
> >
> > - ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
> > - if (ret) {
> > - dev_err(&ofdev->dev, "can't get IO base\n");
> > - return ret;
> > - }
> > + io_res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
> > + fun->io_base = devm_ioremap_resource(&ofdev->dev, io_res);
>
> Why not even using devm_platform_ioremap_resource() resource directly?
Because I need to pass the resource to fsl_upm_find().
^ permalink raw reply
* Re: [PATCH 04/10] mtd: rawnand: fsl_upm: Use devm_kasprintf() to allocate the MTD name
From: Boris Brezillon @ 2020-06-03 13:59 UTC (permalink / raw)
To: Miquel Raynal
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603155531.1b43ddb7@xps13>
On Wed, 3 Jun 2020 15:55:31 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
> 2020 15:49:16 +0200:
>
> > This simplifies the init() error path and the remove() handler.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> > drivers/mtd/nand/raw/fsl_upm.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> > index 9cf79c62ef22..a3e3a968891d 100644
> > --- a/drivers/mtd/nand/raw/fsl_upm.c
> > +++ b/drivers/mtd/nand/raw/fsl_upm.c
> > @@ -176,8 +176,9 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> > return -ENODEV;
> >
> > nand_set_flash_node(&fun->chip, flash_np);
> > - mtd->name = kasprintf(GFP_KERNEL, "0x%llx.%pOFn", (u64)io_res->start,
> > - flash_np);
> > + mtd->name = devm_kasprintf(fun->dev, GFP_KERNEL, "0x%llx.%pOFn",
> > + (u64)io_res->start,
> > + flash_np);
>
> Shouldn't we check if mtd->name was not already set by
> nand_set_flash_node() first?
>
We could, but let's see if we find someone to test those changes first.
^ permalink raw reply
* Re: [PATCH 06/10] mtd: rawnand: fsl_upm: Use gpio descriptors
From: Miquel Raynal @ 2020-06-03 13:59 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-7-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:18 +0200:
> The integer-based GPIO ids are now deprecated in favor of the GPIO desc
> API. The PPC platforms have already been converted to GPIOLIB, so let's
> use gpio descs in the NAND driver too.
>
> While at it, we use devm_gpiod_get_index_optional() so we can get rid
> of the manual gpio desc release done in the init error path and in the
> remove function.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 44 ++++++++--------------------------
> 1 file changed, 10 insertions(+), 34 deletions(-)
>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply
* Re: [PATCH 02/10] mtd: rawnand: fsl_upm: Get rid of the unused fsl_upm_nand.parts field
From: Miquel Raynal @ 2020-06-03 13:50 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-3-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:14 +0200:
> fsl_upm_nand.parts is unused, let's get rid of it.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 76d1032cd35e..6eba2f4a2f5a 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -29,7 +29,6 @@ struct fsl_upm_nand {
> struct device *dev;
> struct nand_chip chip;
> int last_ctrl;
> - struct mtd_partition *parts;
> struct fsl_upm upm;
> uint8_t upm_addr_offset;
> uint8_t upm_cmd_offset;
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply
* Re: [PATCH 01/10] mtd: rawnand: fsl_upm: Remove unused mtd var
From: Miquel Raynal @ 2020-06-03 13:50 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-2-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:13 +0200:
> The mtd var in fun_wait_rnb() is now unused, let's get rid of it and
> fix the warning resulting from this unused var.
>
> Fixes: 50a487e7719c ("mtd: rawnand: Pass a nand_chip object to chip->dev_ready()")
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 627deb26db51..76d1032cd35e 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -62,7 +62,6 @@ static int fun_chip_ready(struct nand_chip *chip)
> static void fun_wait_rnb(struct fsl_upm_nand *fun)
> {
> if (fun->rnb_gpio[fun->mchip_number] >= 0) {
> - struct mtd_info *mtd = nand_to_mtd(&fun->chip);
> int cnt = 1000000;
>
> while (--cnt && !fun_chip_ready(&fun->chip))
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
^ permalink raw reply
* Re: [PATCH 05/10] mtd: rawnand: fsl_upm: Use platform_get_resource() + devm_ioremap_resource()
From: Miquel Raynal @ 2020-06-03 13:58 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-6-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:17 +0200:
> Replace the of_address_to_resource() + devm_ioremap() calls by
> platform_get_resource() + devm_ioremap_resource() ones which allows us
> to get rid of one error message since devm_ioremap_resource() already
> takes care of that.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 23 +++++++----------------
> 1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index a3e3a968891d..54851e9ea784 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -14,7 +14,6 @@
> #include <linux/mtd/nand_ecc.h>
> #include <linux/mtd/partitions.h>
> #include <linux/mtd/mtd.h>
> -#include <linux/of_address.h>
> #include <linux/of_platform.h>
> #include <linux/of_gpio.h>
> #include <linux/io.h>
> @@ -197,7 +196,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> static int fun_probe(struct platform_device *ofdev)
> {
> struct fsl_upm_nand *fun;
> - struct resource io_res;
> + struct resource *io_res;
> const __be32 *prop;
> int rnb_gpio;
> int ret;
> @@ -208,13 +207,12 @@ static int fun_probe(struct platform_device *ofdev)
> if (!fun)
> return -ENOMEM;
>
> - ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
> - if (ret) {
> - dev_err(&ofdev->dev, "can't get IO base\n");
> - return ret;
> - }
> + io_res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
> + fun->io_base = devm_ioremap_resource(&ofdev->dev, io_res);
Why not even using devm_platform_ioremap_resource() resource directly?
^ permalink raw reply
* [PATCH 10/10] dt-bindings: mtd: fsl-upm-nand: Deprecate chip-delay and fsl, upm-wait-flags
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
Those properties are no longer parsed by the driver which is being passed
those information by the core now. Let's deprecate them.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
index fce4894f5a98..25f07c1f9e44 100644
--- a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
@@ -7,14 +7,16 @@ Required properties:
- fsl,upm-cmd-offset : UPM pattern offset for the command latch.
Optional properties:
-- fsl,upm-wait-flags : add chip-dependent short delays after running the
- UPM pattern (0x1), after writing a data byte (0x2) or after
- writing out a buffer (0x4).
- fsl,upm-addr-line-cs-offsets : address offsets for multi-chip support.
The corresponding address lines are used to select the chip.
- gpios : may specify optional GPIOs connected to the Ready-Not-Busy pins
(R/B#). For multi-chip devices, "n" GPIO definitions are required
according to the number of chips.
+
+Deprecated properties:
+- fsl,upm-wait-flags : add chip-dependent short delays after running the
+ UPM pattern (0x1), after writing a data byte (0x2) or after
+ writing out a buffer (0x4).
- chip-delay : chip dependent delay for transferring data from array to
read registers (tR). Required if property "gpios" is not used
(R/B# pins not connected).
@@ -52,8 +54,6 @@ upm@3,0 {
fsl,upm-cmd-offset = <0x08>;
/* Multi-chip NAND device */
fsl,upm-addr-line-cs-offsets = <0x0 0x200>;
- fsl,upm-wait-flags = <0x5>;
- chip-delay = <25>; // in micro-seconds
nand@0 {
#address-cells = <1>;
--
2.25.4
^ permalink raw reply related
* [PATCH 04/10] mtd: rawnand: fsl_upm: Use devm_kasprintf() to allocate the MTD name
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
This simplifies the init() error path and the remove() handler.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 9cf79c62ef22..a3e3a968891d 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -176,8 +176,9 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
return -ENODEV;
nand_set_flash_node(&fun->chip, flash_np);
- mtd->name = kasprintf(GFP_KERNEL, "0x%llx.%pOFn", (u64)io_res->start,
- flash_np);
+ mtd->name = devm_kasprintf(fun->dev, GFP_KERNEL, "0x%llx.%pOFn",
+ (u64)io_res->start,
+ flash_np);
if (!mtd->name) {
ret = -ENOMEM;
goto err;
@@ -190,8 +191,6 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
ret = mtd_device_register(mtd, NULL, 0);
err:
of_node_put(flash_np);
- if (ret)
- kfree(mtd->name);
return ret;
}
@@ -318,7 +317,6 @@ static int fun_remove(struct platform_device *ofdev)
ret = mtd_device_unregister(mtd);
WARN_ON(ret);
nand_cleanup(chip);
- kfree(mtd->name);
for (i = 0; i < fun->mchip_count; i++) {
if (fun->rnb_gpio[i] < 0)
--
2.25.4
^ permalink raw reply related
* [PATCH 03/10] mtd: rawnand: fsl_upm: Allocate the fsl_upm_nand object using devm_kzalloc()
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
This simplifies the init error patch and remove function.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 6eba2f4a2f5a..9cf79c62ef22 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -205,36 +205,34 @@ static int fun_probe(struct platform_device *ofdev)
int size;
int i;
- fun = kzalloc(sizeof(*fun), GFP_KERNEL);
+ fun = devm_kzalloc(&ofdev->dev, sizeof(*fun), GFP_KERNEL);
if (!fun)
return -ENOMEM;
ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
if (ret) {
dev_err(&ofdev->dev, "can't get IO base\n");
- goto err1;
+ return ret;
}
ret = fsl_upm_find(io_res.start, &fun->upm);
if (ret) {
dev_err(&ofdev->dev, "can't find UPM\n");
- goto err1;
+ return ret;
}
prop = of_get_property(ofdev->dev.of_node, "fsl,upm-addr-offset",
&size);
if (!prop || size != sizeof(uint32_t)) {
dev_err(&ofdev->dev, "can't get UPM address offset\n");
- ret = -EINVAL;
- goto err1;
+ return -EINVAL;
}
fun->upm_addr_offset = *prop;
prop = of_get_property(ofdev->dev.of_node, "fsl,upm-cmd-offset", &size);
if (!prop || size != sizeof(uint32_t)) {
dev_err(&ofdev->dev, "can't get UPM command offset\n");
- ret = -EINVAL;
- goto err1;
+ return -EINVAL;
}
fun->upm_cmd_offset = *prop;
@@ -244,7 +242,7 @@ static int fun_probe(struct platform_device *ofdev)
fun->mchip_count = size / sizeof(uint32_t);
if (fun->mchip_count >= NAND_MAX_CHIPS) {
dev_err(&ofdev->dev, "too much multiple chips\n");
- goto err1;
+ return -EINVAL;
}
for (i = 0; i < fun->mchip_count; i++)
fun->mchip_offsets[i] = be32_to_cpu(prop[i]);
@@ -306,8 +304,6 @@ static int fun_probe(struct platform_device *ofdev)
break;
gpio_free(fun->rnb_gpio[i]);
}
-err1:
- kfree(fun);
return ret;
}
@@ -330,8 +326,6 @@ static int fun_remove(struct platform_device *ofdev)
gpio_free(fun->rnb_gpio[i]);
}
- kfree(fun);
-
return 0;
}
--
2.25.4
^ permalink raw reply related
* [PATCH 08/10] mtd: rawnand: fsl_upm: Implement exec_op()
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
Implement exec_op() so we can get rid of the legacy interface
implementation.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 86 ++++++++++++++++++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 9a63e36825d8..03ca20930274 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -194,6 +194,91 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
return ret;
}
+static int func_exec_instr(struct nand_chip *chip,
+ const struct nand_op_instr *instr)
+{
+ struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
+ u32 mar, reg_offs = fun->mchip_offsets[fun->mchip_number];
+ unsigned int i;
+ const u8 *out;
+ u8 *in;
+
+ switch (instr->type) {
+ case NAND_OP_CMD_INSTR:
+ fsl_upm_start_pattern(&fun->upm, fun->upm_cmd_offset);
+ mar = (instr->ctx.cmd.opcode << (32 - fun->upm.width)) |
+ reg_offs;
+ fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
+ fsl_upm_end_pattern(&fun->upm);
+ return 0;
+
+ case NAND_OP_ADDR_INSTR:
+ fsl_upm_start_pattern(&fun->upm, fun->upm_addr_offset);
+ for (i = 0; i < instr->ctx.addr.naddrs; i++) {
+ mar = (instr->ctx.addr.addrs[i] << (32 - fun->upm.width)) |
+ reg_offs;
+ fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
+ }
+ fsl_upm_end_pattern(&fun->upm);
+ return 0;
+
+ case NAND_OP_DATA_IN_INSTR:
+ in = instr->ctx.data.buf.in;
+ for (i = 0; i < instr->ctx.data.len; i++)
+ in[i] = in_8(fun->io_base + reg_offs);
+ return 0;
+
+ case NAND_OP_DATA_OUT_INSTR:
+ out = instr->ctx.data.buf.out;
+ for (i = 0; i < instr->ctx.data.len; i++)
+ out_8(fun->io_base + reg_offs, out[i]);
+ return 0;
+
+ case NAND_OP_WAITRDY_INSTR:
+ if (!fun->rnb_gpio[fun->mchip_number])
+ return nand_soft_waitrdy(chip, instr->ctx.waitrdy.timeout_ms);
+
+ return nand_gpio_waitrdy(chip, fun->rnb_gpio[fun->mchip_number],
+ instr->ctx.waitrdy.timeout_ms);
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int fun_exec_op(struct nand_chip *chip, const struct nand_operation *op,
+ bool check_only)
+{
+ struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
+ unsigned int i;
+ int ret;
+
+ if (op->cs > NAND_MAX_CHIPS)
+ return -EINVAL;
+
+ if (check_only)
+ return 0;
+
+ fun->mchip_number = op->cs;
+
+ for (i = 0; i < op->ninstrs; i++) {
+ ret = func_exec_instr(chip, &op->instrs[i]);
+ if (ret)
+ return ret;
+
+ if (op->instrs[i].delay_ns)
+ ndelay(op->instrs[i].delay_ns);
+ }
+
+ return 0;
+}
+
+static const struct nand_controller_ops fun_ops = {
+ .exec_op = fun_exec_op,
+};
+
static int fun_probe(struct platform_device *ofdev)
{
struct fsl_upm_nand *fun;
@@ -271,6 +356,7 @@ static int fun_probe(struct platform_device *ofdev)
FSL_UPM_WAIT_WRITE_BYTE;
nand_controller_init(&fun->base);
+ fun->base.ops = &fun_ops;
fun->dev = &ofdev->dev;
fun->last_ctrl = NAND_CLE;
--
2.25.4
^ permalink raw reply related
* [PATCH 07/10] mtd: rawnand: fsl_upm: Inherit from nand_controller
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
Explicitly inherit from nand_controller instead of relying on the
nand_chip.legacy.dummy_controller field.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 977b7aad419b..9a63e36825d8 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -24,6 +24,7 @@
#define FSL_UPM_WAIT_WRITE_BUFFER 0x4
struct fsl_upm_nand {
+ struct nand_controller base;
struct device *dev;
struct nand_chip chip;
int last_ctrl;
@@ -167,6 +168,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
if (!fun->rnb_gpio[0])
fun->chip.legacy.dev_ready = fun_chip_ready;
+ fun->chip.controller = &fun->base;
mtd->dev.parent = fun->dev;
flash_np = of_get_next_child(upm_np, NULL);
@@ -268,6 +270,7 @@ static int fun_probe(struct platform_device *ofdev)
fun->wait_flags = FSL_UPM_WAIT_RUN_PATTERN |
FSL_UPM_WAIT_WRITE_BYTE;
+ nand_controller_init(&fun->base);
fun->dev = &ofdev->dev;
fun->last_ctrl = NAND_CLE;
--
2.25.4
^ permalink raw reply related
* [PATCH 01/10] mtd: rawnand: fsl_upm: Remove unused mtd var
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
The mtd var in fun_wait_rnb() is now unused, let's get rid of it and
fix the warning resulting from this unused var.
Fixes: 50a487e7719c ("mtd: rawnand: Pass a nand_chip object to chip->dev_ready()")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 627deb26db51..76d1032cd35e 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -62,7 +62,6 @@ static int fun_chip_ready(struct nand_chip *chip)
static void fun_wait_rnb(struct fsl_upm_nand *fun)
{
if (fun->rnb_gpio[fun->mchip_number] >= 0) {
- struct mtd_info *mtd = nand_to_mtd(&fun->chip);
int cnt = 1000000;
while (--cnt && !fun_chip_ready(&fun->chip))
--
2.25.4
^ permalink raw reply related
* [PATCH 09/10] mtd: rawnand: fsl_upm: Get rid of the legacy interface implementation
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
Now that the driver implements exec_op(), we can get rid of the legacy
interface implementation.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 133 ---------------------------------
1 file changed, 133 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 03ca20930274..197850aeb261 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -19,15 +19,10 @@
#include <linux/slab.h>
#include <asm/fsl_lbc.h>
-#define FSL_UPM_WAIT_RUN_PATTERN 0x1
-#define FSL_UPM_WAIT_WRITE_BYTE 0x2
-#define FSL_UPM_WAIT_WRITE_BUFFER 0x4
-
struct fsl_upm_nand {
struct nand_controller base;
struct device *dev;
struct nand_chip chip;
- int last_ctrl;
struct fsl_upm upm;
uint8_t upm_addr_offset;
uint8_t upm_cmd_offset;
@@ -36,8 +31,6 @@ struct fsl_upm_nand {
uint32_t mchip_offsets[NAND_MAX_CHIPS];
uint32_t mchip_count;
uint32_t mchip_number;
- int chip_delay;
- uint32_t wait_flags;
};
static inline struct fsl_upm_nand *to_fsl_upm_nand(struct mtd_info *mtdinfo)
@@ -46,105 +39,6 @@ static inline struct fsl_upm_nand *to_fsl_upm_nand(struct mtd_info *mtdinfo)
chip);
}
-static int fun_chip_ready(struct nand_chip *chip)
-{
- struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
-
- if (gpiod_get_value(fun->rnb_gpio[fun->mchip_number]))
- return 1;
-
- dev_vdbg(fun->dev, "busy\n");
- return 0;
-}
-
-static void fun_wait_rnb(struct fsl_upm_nand *fun)
-{
- if (fun->rnb_gpio[fun->mchip_number] >= 0) {
- int cnt = 1000000;
-
- while (--cnt && !fun_chip_ready(&fun->chip))
- cpu_relax();
- if (!cnt)
- dev_err(fun->dev, "tired waiting for RNB\n");
- } else {
- ndelay(100);
- }
-}
-
-static void fun_cmd_ctrl(struct nand_chip *chip, int cmd, unsigned int ctrl)
-{
- struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
- u32 mar;
-
- if (!(ctrl & fun->last_ctrl)) {
- fsl_upm_end_pattern(&fun->upm);
-
- if (cmd == NAND_CMD_NONE)
- return;
-
- fun->last_ctrl = ctrl & (NAND_ALE | NAND_CLE);
- }
-
- if (ctrl & NAND_CTRL_CHANGE) {
- if (ctrl & NAND_ALE)
- fsl_upm_start_pattern(&fun->upm, fun->upm_addr_offset);
- else if (ctrl & NAND_CLE)
- fsl_upm_start_pattern(&fun->upm, fun->upm_cmd_offset);
- }
-
- mar = (cmd << (32 - fun->upm.width)) |
- fun->mchip_offsets[fun->mchip_number];
- fsl_upm_run_pattern(&fun->upm, chip->legacy.IO_ADDR_R, mar);
-
- if (fun->wait_flags & FSL_UPM_WAIT_RUN_PATTERN)
- fun_wait_rnb(fun);
-}
-
-static void fun_select_chip(struct nand_chip *chip, int mchip_nr)
-{
- struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
-
- if (mchip_nr == -1) {
- chip->legacy.cmd_ctrl(chip, NAND_CMD_NONE, 0 | NAND_CTRL_CHANGE);
- } else if (mchip_nr >= 0 && mchip_nr < NAND_MAX_CHIPS) {
- fun->mchip_number = mchip_nr;
- chip->legacy.IO_ADDR_R = fun->io_base + fun->mchip_offsets[mchip_nr];
- chip->legacy.IO_ADDR_W = chip->legacy.IO_ADDR_R;
- } else {
- BUG();
- }
-}
-
-static uint8_t fun_read_byte(struct nand_chip *chip)
-{
- struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
-
- return in_8(fun->chip.legacy.IO_ADDR_R);
-}
-
-static void fun_read_buf(struct nand_chip *chip, uint8_t *buf, int len)
-{
- struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
- int i;
-
- for (i = 0; i < len; i++)
- buf[i] = in_8(fun->chip.legacy.IO_ADDR_R);
-}
-
-static void fun_write_buf(struct nand_chip *chip, const uint8_t *buf, int len)
-{
- struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
- int i;
-
- for (i = 0; i < len; i++) {
- out_8(fun->chip.legacy.IO_ADDR_W, buf[i]);
- if (fun->wait_flags & FSL_UPM_WAIT_WRITE_BYTE)
- fun_wait_rnb(fun);
- }
- if (fun->wait_flags & FSL_UPM_WAIT_WRITE_BUFFER)
- fun_wait_rnb(fun);
-}
-
static int fun_chip_init(struct fsl_upm_nand *fun,
const struct device_node *upm_np,
const struct resource *io_res)
@@ -153,21 +47,8 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
int ret;
struct device_node *flash_np;
- fun->chip.legacy.IO_ADDR_R = fun->io_base;
- fun->chip.legacy.IO_ADDR_W = fun->io_base;
- fun->chip.legacy.cmd_ctrl = fun_cmd_ctrl;
- fun->chip.legacy.chip_delay = fun->chip_delay;
- fun->chip.legacy.read_byte = fun_read_byte;
- fun->chip.legacy.read_buf = fun_read_buf;
- fun->chip.legacy.write_buf = fun_write_buf;
fun->chip.ecc.mode = NAND_ECC_SOFT;
fun->chip.ecc.algo = NAND_ECC_HAMMING;
- if (fun->mchip_count > 1)
- fun->chip.legacy.select_chip = fun_select_chip;
-
- if (!fun->rnb_gpio[0])
- fun->chip.legacy.dev_ready = fun_chip_ready;
-
fun->chip.controller = &fun->base;
mtd->dev.parent = fun->dev;
@@ -342,23 +223,9 @@ static int fun_probe(struct platform_device *ofdev)
}
}
- prop = of_get_property(ofdev->dev.of_node, "chip-delay", NULL);
- if (prop)
- fun->chip_delay = be32_to_cpup(prop);
- else
- fun->chip_delay = 50;
-
- prop = of_get_property(ofdev->dev.of_node, "fsl,upm-wait-flags", &size);
- if (prop && size == sizeof(uint32_t))
- fun->wait_flags = be32_to_cpup(prop);
- else
- fun->wait_flags = FSL_UPM_WAIT_RUN_PATTERN |
- FSL_UPM_WAIT_WRITE_BYTE;
-
nand_controller_init(&fun->base);
fun->base.ops = &fun_ops;
fun->dev = &ofdev->dev;
- fun->last_ctrl = NAND_CLE;
ret = fun_chip_init(fun, ofdev->dev.of_node, io_res);
if (ret)
--
2.25.4
^ permalink raw reply related
* [PATCH 00/10] mtd: rawnand: fsl_upm: Convert to exec_op() (and more)
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
Hello,
A bit of context to explain the motivation behind those conversions
I've been sending for the last few weeks. The raw NAND subsystem
carries a lot of history which makes any rework not only painful, but
also subject to regressions which we only detect when someone dares to
update its kernel on one of those ancient HW. While carrying drivers
for old HW is not a problem per se, carrying ancient and unmaintained
drivers that are not converted to new APIs is a maintenance burden,
hence this massive conversion attempt I'm conducting here.
So here is a series converting the FSM UPM NAND controller driver to
exec_op(), plus a bunch of minor improvements done along the way.
I hope I'll find someone to test those changes, but if there's no one
still having access to this HW or no interest in keeping it supported
in recent kernel versions, we should definitely consider removing the
driver instead.
Regards,
Boris
Boris Brezillon (10):
mtd: rawnand: fsl_upm: Remove unused mtd var
mtd: rawnand: fsl_upm: Get rid of the unused fsl_upm_nand.parts field
mtd: rawnand: fsl_upm: Allocate the fsl_upm_nand object using
devm_kzalloc()
mtd: rawnand: fsl_upm: Use devm_kasprintf() to allocate the MTD name
mtd: rawnand: fsl_upm: Use platform_get_resource() +
devm_ioremap_resource()
mtd: rawnand: fsl_upm: Use gpio descriptors
mtd: rawnand: fsl_upm: Inherit from nand_controller
mtd: rawnand: fsl_upm: Implement exec_op()
mtd: rawnand: fsl_upm: Get rid of the legacy interface implementation
dt-bindings: mtd: fsl-upm-nand: Deprecate chip-delay and
fsl,upm-wait-flags
.../devicetree/bindings/mtd/fsl-upm-nand.txt | 10 +-
drivers/mtd/nand/raw/fsl_upm.c | 311 +++++++-----------
2 files changed, 117 insertions(+), 204 deletions(-)
--
2.25.4
^ permalink raw reply
* [PATCH 02/10] mtd: rawnand: fsl_upm: Get rid of the unused fsl_upm_nand.parts field
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
fsl_upm_nand.parts is unused, let's get rid of it.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 76d1032cd35e..6eba2f4a2f5a 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -29,7 +29,6 @@ struct fsl_upm_nand {
struct device *dev;
struct nand_chip chip;
int last_ctrl;
- struct mtd_partition *parts;
struct fsl_upm upm;
uint8_t upm_addr_offset;
uint8_t upm_cmd_offset;
--
2.25.4
^ permalink raw reply related
* [PATCH 06/10] mtd: rawnand: fsl_upm: Use gpio descriptors
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
The integer-based GPIO ids are now deprecated in favor of the GPIO desc
API. The PPC platforms have already been converted to GPIOLIB, so let's
use gpio descs in the NAND driver too.
While at it, we use devm_gpiod_get_index_optional() so we can get rid
of the manual gpio desc release done in the init error path and in the
remove function.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 44 ++++++++--------------------------
1 file changed, 10 insertions(+), 34 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 54851e9ea784..977b7aad419b 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -15,7 +15,6 @@
#include <linux/mtd/partitions.h>
#include <linux/mtd/mtd.h>
#include <linux/of_platform.h>
-#include <linux/of_gpio.h>
#include <linux/io.h>
#include <linux/slab.h>
#include <asm/fsl_lbc.h>
@@ -32,7 +31,7 @@ struct fsl_upm_nand {
uint8_t upm_addr_offset;
uint8_t upm_cmd_offset;
void __iomem *io_base;
- int rnb_gpio[NAND_MAX_CHIPS];
+ struct gpio_desc *rnb_gpio[NAND_MAX_CHIPS];
uint32_t mchip_offsets[NAND_MAX_CHIPS];
uint32_t mchip_count;
uint32_t mchip_number;
@@ -50,7 +49,7 @@ static int fun_chip_ready(struct nand_chip *chip)
{
struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
- if (gpio_get_value(fun->rnb_gpio[fun->mchip_number]))
+ if (gpiod_get_value(fun->rnb_gpio[fun->mchip_number]))
return 1;
dev_vdbg(fun->dev, "busy\n");
@@ -165,7 +164,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
if (fun->mchip_count > 1)
fun->chip.legacy.select_chip = fun_select_chip;
- if (fun->rnb_gpio[0] >= 0)
+ if (!fun->rnb_gpio[0])
fun->chip.legacy.dev_ready = fun_chip_ready;
mtd->dev.parent = fun->dev;
@@ -198,7 +197,6 @@ static int fun_probe(struct platform_device *ofdev)
struct fsl_upm_nand *fun;
struct resource *io_res;
const __be32 *prop;
- int rnb_gpio;
int ret;
int size;
int i;
@@ -248,20 +246,12 @@ static int fun_probe(struct platform_device *ofdev)
}
for (i = 0; i < fun->mchip_count; i++) {
- fun->rnb_gpio[i] = -1;
- rnb_gpio = of_get_gpio(ofdev->dev.of_node, i);
- if (rnb_gpio >= 0) {
- ret = gpio_request(rnb_gpio, dev_name(&ofdev->dev));
- if (ret) {
- dev_err(&ofdev->dev,
- "can't request RNB gpio #%d\n", i);
- goto err2;
- }
- gpio_direction_input(rnb_gpio);
- fun->rnb_gpio[i] = rnb_gpio;
- } else if (rnb_gpio == -EINVAL) {
+ fun->rnb_gpio[i] = devm_gpiod_get_index_optional(&ofdev->dev,
+ NULL, i,
+ GPIOD_IN);
+ if (IS_ERR(fun->rnb_gpio[i])) {
dev_err(&ofdev->dev, "RNB gpio #%d is invalid\n", i);
- goto err2;
+ return PTR_ERR(fun->rnb_gpio[i]);
}
}
@@ -283,19 +273,11 @@ static int fun_probe(struct platform_device *ofdev)
ret = fun_chip_init(fun, ofdev->dev.of_node, io_res);
if (ret)
- goto err2;
+ return ret;
dev_set_drvdata(&ofdev->dev, fun);
return 0;
-err2:
- for (i = 0; i < fun->mchip_count; i++) {
- if (fun->rnb_gpio[i] < 0)
- break;
- gpio_free(fun->rnb_gpio[i]);
- }
-
- return ret;
}
static int fun_remove(struct platform_device *ofdev)
@@ -303,18 +285,12 @@ static int fun_remove(struct platform_device *ofdev)
struct fsl_upm_nand *fun = dev_get_drvdata(&ofdev->dev);
struct nand_chip *chip = &fun->chip;
struct mtd_info *mtd = nand_to_mtd(chip);
- int ret, i;
+ int ret;
ret = mtd_device_unregister(mtd);
WARN_ON(ret);
nand_cleanup(chip);
- for (i = 0; i < fun->mchip_count; i++) {
- if (fun->rnb_gpio[i] < 0)
- break;
- gpio_free(fun->rnb_gpio[i]);
- }
-
return 0;
}
--
2.25.4
^ permalink raw reply related
* [PATCH 05/10] mtd: rawnand: fsl_upm: Use platform_get_resource() + devm_ioremap_resource()
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
To: Anton Vorontsov, Miquel Raynal, linux-mtd
Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>
Replace the of_address_to_resource() + devm_ioremap() calls by
platform_get_resource() + devm_ioremap_resource() ones which allows us
to get rid of one error message since devm_ioremap_resource() already
takes care of that.
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
drivers/mtd/nand/raw/fsl_upm.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index a3e3a968891d..54851e9ea784 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -14,7 +14,6 @@
#include <linux/mtd/nand_ecc.h>
#include <linux/mtd/partitions.h>
#include <linux/mtd/mtd.h>
-#include <linux/of_address.h>
#include <linux/of_platform.h>
#include <linux/of_gpio.h>
#include <linux/io.h>
@@ -197,7 +196,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
static int fun_probe(struct platform_device *ofdev)
{
struct fsl_upm_nand *fun;
- struct resource io_res;
+ struct resource *io_res;
const __be32 *prop;
int rnb_gpio;
int ret;
@@ -208,13 +207,12 @@ static int fun_probe(struct platform_device *ofdev)
if (!fun)
return -ENOMEM;
- ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
- if (ret) {
- dev_err(&ofdev->dev, "can't get IO base\n");
- return ret;
- }
+ io_res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
+ fun->io_base = devm_ioremap_resource(&ofdev->dev, io_res);
+ if (IS_ERR(fun->io_base))
+ return PTR_ERR(fun->io_base);
- ret = fsl_upm_find(io_res.start, &fun->upm);
+ ret = fsl_upm_find(io_res->start, &fun->upm);
if (ret) {
dev_err(&ofdev->dev, "can't find UPM\n");
return ret;
@@ -280,17 +278,10 @@ static int fun_probe(struct platform_device *ofdev)
fun->wait_flags = FSL_UPM_WAIT_RUN_PATTERN |
FSL_UPM_WAIT_WRITE_BYTE;
- fun->io_base = devm_ioremap(&ofdev->dev, io_res.start,
- resource_size(&io_res));
- if (!fun->io_base) {
- ret = -ENOMEM;
- goto err2;
- }
-
fun->dev = &ofdev->dev;
fun->last_ctrl = NAND_CLE;
- ret = fun_chip_init(fun, ofdev->dev.of_node, &io_res);
+ ret = fun_chip_init(fun, ofdev->dev.of_node, io_res);
if (ret)
goto err2;
--
2.25.4
^ permalink raw reply related
* Re: [PATCH 04/10] mtd: rawnand: fsl_upm: Use devm_kasprintf() to allocate the MTD name
From: Miquel Raynal @ 2020-06-03 13:55 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-5-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:16 +0200:
> This simplifies the init() error path and the remove() handler.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 9cf79c62ef22..a3e3a968891d 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -176,8 +176,9 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> return -ENODEV;
>
> nand_set_flash_node(&fun->chip, flash_np);
> - mtd->name = kasprintf(GFP_KERNEL, "0x%llx.%pOFn", (u64)io_res->start,
> - flash_np);
> + mtd->name = devm_kasprintf(fun->dev, GFP_KERNEL, "0x%llx.%pOFn",
> + (u64)io_res->start,
> + flash_np);
Shouldn't we check if mtd->name was not already set by
nand_set_flash_node() first?
^ permalink raw reply
* Re: [PATCH 03/10] mtd: rawnand: fsl_upm: Allocate the fsl_upm_nand object using devm_kzalloc()
From: Miquel Raynal @ 2020-06-03 13:52 UTC (permalink / raw)
To: Boris Brezillon
Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-4-boris.brezillon@collabora.com>
Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 3 Jun
2020 15:49:15 +0200:
> This simplifies the init error patch and remove function.
path?
Otherwise:
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
>
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
> drivers/mtd/nand/raw/fsl_upm.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 6eba2f4a2f5a..9cf79c62ef22 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -205,36 +205,34 @@ static int fun_probe(struct platform_device *ofdev)
> int size;
> int i;
>
> - fun = kzalloc(sizeof(*fun), GFP_KERNEL);
> + fun = devm_kzalloc(&ofdev->dev, sizeof(*fun), GFP_KERNEL);
> if (!fun)
> return -ENOMEM;
>
> ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
> if (ret) {
> dev_err(&ofdev->dev, "can't get IO base\n");
> - goto err1;
> + return ret;
> }
>
> ret = fsl_upm_find(io_res.start, &fun->upm);
> if (ret) {
> dev_err(&ofdev->dev, "can't find UPM\n");
> - goto err1;
> + return ret;
> }
>
> prop = of_get_property(ofdev->dev.of_node, "fsl,upm-addr-offset",
> &size);
> if (!prop || size != sizeof(uint32_t)) {
> dev_err(&ofdev->dev, "can't get UPM address offset\n");
> - ret = -EINVAL;
> - goto err1;
> + return -EINVAL;
> }
> fun->upm_addr_offset = *prop;
>
> prop = of_get_property(ofdev->dev.of_node, "fsl,upm-cmd-offset", &size);
> if (!prop || size != sizeof(uint32_t)) {
> dev_err(&ofdev->dev, "can't get UPM command offset\n");
> - ret = -EINVAL;
> - goto err1;
> + return -EINVAL;
> }
> fun->upm_cmd_offset = *prop;
>
> @@ -244,7 +242,7 @@ static int fun_probe(struct platform_device *ofdev)
> fun->mchip_count = size / sizeof(uint32_t);
> if (fun->mchip_count >= NAND_MAX_CHIPS) {
> dev_err(&ofdev->dev, "too much multiple chips\n");
> - goto err1;
> + return -EINVAL;
> }
> for (i = 0; i < fun->mchip_count; i++)
> fun->mchip_offsets[i] = be32_to_cpu(prop[i]);
> @@ -306,8 +304,6 @@ static int fun_probe(struct platform_device *ofdev)
> break;
> gpio_free(fun->rnb_gpio[i]);
> }
> -err1:
> - kfree(fun);
>
> return ret;
> }
> @@ -330,8 +326,6 @@ static int fun_remove(struct platform_device *ofdev)
> gpio_free(fun->rnb_gpio[i]);
> }
>
> - kfree(fun);
> -
> return 0;
> }
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox