Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935883AbcJQQ0w (ORCPT ); Mon, 17 Oct 2016 12:26:52 -0400 Received: from mx08-00178001.pphosted.com ([91.207.212.93]:59921 "EHLO mx07-00178001.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S934893AbcJQQ0l (ORCPT ); Mon, 17 Oct 2016 12:26:41 -0400 Subject: Re: [PATCH v3 02/20] remoteproc: core: Add function to dump resource table To: Matt Redfearn , , , References: <1476288038-24909-1-git-send-email-loic.pallardy@st.com> <1476288038-24909-3-git-send-email-loic.pallardy@st.com> <3d83f9e3-a04a-914e-88ba-9f94a2e3827b@imgtec.com> CC: , , From: loic pallardy Message-ID: Date: Mon, 17 Oct 2016 18:26:30 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <3d83f9e3-a04a-914e-88ba-9f94a2e3827b@imgtec.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.201.23.23] X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-10-17_07:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5822 Lines: 155 On 10/14/2016 10:37 AM, Matt Redfearn wrote: > Hi Loic, > Hi Matt, > > On 12/10/16 17:00, Loic Pallardy wrote: >> Firmware can be loaded with a resource table, which details >> resources needed by coprocessor like carevout memory, virtual >> device, trace log buffer etc. >> >> Until now, no method exists to display resource table content. >> This function adds the capability to display the different >> resources associated to a firmware if DEBUG is enabled. > > How about instead adding this to a debugfs entry such that one can > always access it rather than having to find it in the kernel log, only > if DEBUG was enabled? It was one of my remarks on V1, but Lee prefers to have this change on the top of this series. Both functions are their interest: - current function help to analyse changes done during boot sequence between original and modified resource table - debugfs function will allow to display resource table provided to coprocessor and understand what could be wrong in case of error. I propose to add a patch on the top of the series to support debugfs interface like mentioned previously by Lee. Regards, Loic > > Thanks, > Matt > >> >> Signed-off-by: Lee Jones >> Signed-off-by: Loic Pallardy >> --- >> drivers/remoteproc/remoteproc_core.c | 85 >> ++++++++++++++++++++++++++++++++++++ >> 1 file changed, 85 insertions(+) >> >> diff --git a/drivers/remoteproc/remoteproc_core.c >> b/drivers/remoteproc/remoteproc_core.c >> index 67633ee..3c8395b 100644 >> --- a/drivers/remoteproc/remoteproc_core.c >> +++ b/drivers/remoteproc/remoteproc_core.c >> @@ -785,6 +785,91 @@ static void rproc_resource_cleanup(struct rproc >> *rproc) >> rproc_remove_virtio_dev(rvdev); >> } >> +static void rproc_dump_resource_table(struct rproc *rproc, >> + struct resource_table *table, int size) >> +{ >> + static const char *types[] = {"carveout", "devmem", "trace", >> "vdev"}; >> + struct device *dev = &rproc->dev; >> + struct fw_rsc_carveout *c; >> + struct fw_rsc_devmem *d; >> + struct fw_rsc_trace *t; >> + struct fw_rsc_vdev *v; >> + int i, j; >> + >> + if (!table) { >> + dev_dbg(dev, "No resource table found\n"); >> + return; >> + } >> + >> + dev_dbg(dev, "Resource Table: Version %d with %d entries [size: >> %x]\n", >> + table->ver, table->num, size); >> + >> + for (i = 0; i < table->num; i++) { >> + int offset = table->offset[i]; >> + struct fw_rsc_hdr *hdr = (void *)table + offset; >> + void *rsc = (void *)hdr + sizeof(*hdr); >> + >> + switch (hdr->type) { >> + case RSC_CARVEOUT: >> + c = rsc; >> + dev_dbg(dev, "Entry %d is of type %s\n", i, >> types[hdr->type]); >> + dev_dbg(dev, " Device Address 0x%x\n", c->da); >> + dev_dbg(dev, " Physical Address 0x%x\n", c->pa); >> + dev_dbg(dev, " Length 0x%x Bytes\n", c->len); >> + dev_dbg(dev, " Flags 0x%x\n", c->flags); >> + dev_dbg(dev, " Reserved (should be zero) [%d]\n", >> c->reserved); >> + dev_dbg(dev, " Name %s\n\n", c->name); >> + break; >> + case RSC_DEVMEM: >> + d = rsc; >> + dev_dbg(dev, "Entry %d is of type %s\n", i, >> types[hdr->type]); >> + dev_dbg(dev, " Device Address 0x%x\n", d->da); >> + dev_dbg(dev, " Physical Address 0x%x\n", d->pa); >> + dev_dbg(dev, " Length 0x%x Bytes\n", d->len); >> + dev_dbg(dev, " Flags 0x%x\n", d->flags); >> + dev_dbg(dev, " Reserved (should be zero) [%d]\n", >> d->reserved); >> + dev_dbg(dev, " Name %s\n\n", d->name); >> + break; >> + case RSC_TRACE: >> + t = rsc; >> + dev_dbg(dev, "Entry %d is of type %s\n", i, >> types[hdr->type]); >> + dev_dbg(dev, " Device Address 0x%x\n", t->da); >> + dev_dbg(dev, " Length 0x%x Bytes\n", t->len); >> + dev_dbg(dev, " Reserved (should be zero) [%d]\n", >> t->reserved); >> + dev_dbg(dev, " Name %s\n\n", t->name); >> + break; >> + case RSC_VDEV: >> + v = rsc; >> + dev_dbg(dev, "Entry %d is of type %s\n", i, >> types[hdr->type]); >> + >> + dev_dbg(dev, " ID %d\n", v->id); >> + dev_dbg(dev, " Notify ID %d\n", v->notifyid); >> + dev_dbg(dev, " Device features 0x%x\n", v->dfeatures); >> + dev_dbg(dev, " Guest features 0x%x\n", v->gfeatures); >> + dev_dbg(dev, " Config length 0x%x\n", v->config_len); >> + dev_dbg(dev, " Status 0x%x\n", v->status); >> + dev_dbg(dev, " Number of vrings %d\n", v->num_of_vrings); >> + dev_dbg(dev, " Reserved (should be zero) [%d][%d]\n\n", >> + v->reserved[0], v->reserved[1]); >> + >> + for (j = 0; j < v->num_of_vrings; j++) { >> + dev_dbg(dev, " Vring %d\n", j); >> + dev_dbg(dev, " Device Address 0x%x\n", >> v->vring[j].da); >> + dev_dbg(dev, " Alignment %d\n", v->vring[j].align); >> + dev_dbg(dev, " Number of buffers %d\n", >> v->vring[j].num); >> + dev_dbg(dev, " Notify ID %d\n", >> v->vring[j].notifyid); >> + dev_dbg(dev, " Physical Address 0x%x\n\n", >> + v->vring[j].pa); >> + } >> + break; >> + default: >> + dev_dbg(dev, "Invalid resource type found: %d [hdr: %p]\n", >> + hdr->type, hdr); >> + return; >> + } >> + } >> +} >> + >> int rproc_request_resource(struct rproc *rproc, u32 type, void >> *resource) >> { >> struct device *dev = &rproc->dev; >