From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 605ADC433EF for ; Mon, 9 May 2022 13:34:52 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id DBFA683EE0; Mon, 9 May 2022 15:34:49 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=fastree3d.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=fastree3d-com.20210112.gappssmtp.com header.i=@fastree3d-com.20210112.gappssmtp.com header.b="0h1PFASB"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 6828B83E14; Mon, 9 May 2022 15:34:48 +0200 (CEST) Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id C54FA83EFA for ; Mon, 9 May 2022 15:34:42 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=fastree3d.com Authentication-Results: phobos.denx.de; spf=none smtp.mailfrom=adrian.fiergolski@fastree3d.com Received: by mail-lf1-x131.google.com with SMTP id p26so10567914lfh.10 for ; Mon, 09 May 2022 06:34:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fastree3d-com.20210112.gappssmtp.com; s=20210112; h=from:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:in-reply-to :content-transfer-encoding; bh=2GtCfZM9E0Ako6kCPtNSfPFvUDdyFhPT9xtlJRO/DnQ=; b=0h1PFASBFEWSTMSLLTb+ZO+sraAZjIq8d11fU5NdviOxnJiTOWabvVATVCpwRV3bhM fagn56pwPYJmTpsvq3SgOa5cqPMA8CC32Swtzq4pR1XorYbNXeXJmGwIDGM4jL03UBAj 5Jefh/dtVNNUW0EQf0E9J4ed+WIxAHjcp9UJiaaJcpKDX1k4r+SgnORjPC3bD+f4oPoT W9Q8Gglew6jZpqKSwoL3jI7Hu7OyRTlQI+q0+jp2v/JWcEpNR3GN5TIUyMhmB6BI6il7 oWtjfRYnHolyuHTSYOfrHezTkM5ALyma0Tf+QOlbiTB4b73mtuyjuoFMOVYpqyMFyrVt oG+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:message-id:date:mime-version:user-agent :subject:content-language:to:cc:references:in-reply-to :content-transfer-encoding; bh=2GtCfZM9E0Ako6kCPtNSfPFvUDdyFhPT9xtlJRO/DnQ=; b=YaC3fX/kL8tWlqKIcEtf5c1YcX9lRDdF6LZr+avErrU6W9r7WWFGh/xF9Q6AmSA0QF lzs4qTvkXfWRw7y0JjJeA/sKa0PYk4y765/HQYzvdwOmTo+oKuez0JDumC9aaOpxd2ou wC//LIZf21J5RKlzW973PTCH43gppyd+ILOKS7ZspVuKDqbt2phz1HVPKm5HbxOrw/Ky 8DtSZNDrYa8Bp9moLwkeoKHiXrL8goZCBZdMpuONInaVL7Zpgrjo54fcpVP8zDn5UKBz mBRHHE+92l/02rlNGyW+tlgmh3MPTzDrWZEJVksim9+JntUn0NJb6EwUEX+htzRutJmy pKCQ== X-Gm-Message-State: AOAM532gTfVpvk6t1eqM2HqbdrhV1SXHFNnCiGWe4W3lhDs50AF3u867 3zantI8aje6bgODW2YTziwHU X-Google-Smtp-Source: ABdhPJwuPUZyW9WRclMAqbbeJ9nG5JtCLdIm3lAIYNXI2+PeT1/iuJaqIAs5whJBKmDpII9g+6UcKg== X-Received: by 2002:a05:6512:1699:b0:473:edee:20db with SMTP id bu25-20020a056512169900b00473edee20dbmr12358212lfb.316.1652103281856; Mon, 09 May 2022 06:34:41 -0700 (PDT) Received: from [192.168.61.139] (wlan-weiti.elka.pw.edu.pl. [194.29.160.181]) by smtp.gmail.com with ESMTPSA id x33-20020a056512132100b0047255d21122sm1471259lfu.81.2022.05.09.06.34.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 09 May 2022 06:34:41 -0700 (PDT) From: Adrian Fiergolski X-Google-Original-From: Adrian Fiergolski Message-ID: <9bd98f7b-e80a-b14a-862d-ebe84d1ef2c4@fastree3d.com> Date: Mon, 9 May 2022 15:34:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0 Thunderbird/91.9.0 Subject: Re: [PATCH v7 2/7] fpga: add fit_fpga_load function Content-Language: en-GB To: Michal Simek Cc: Oleksandr Suvorov , Ricardo Salveti , Igor Opaniuk , Jorge Ramirez-Ortiz , Alexandru Gagniuc , Bin Meng , Heiko Schocher , Jagan Teki , Klaus Heinrich Kiwi , Sean Anderson , Simon Glass , Steffen Jaeckel , U-Boot Mailing List , Oleksandr Suvorov References: <20220411180046.1505209-1-adrian.fiergolski@fastree3d.com> <20220411180046.1505209-3-adrian.fiergolski@fastree3d.com> <3929af10-3e4e-82bb-b5dc-d75e8d3a27d9@xilinx.com> <49817a87-7de5-811a-131f-a22a6f4c0299@fastree3d.com> <817d8e15-a053-7542-b543-d1cf2999977d@fastree3d.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Michal, On 09.05.2022 15:28, Oleksandr Suvorov wrote: > Hi Adrian, > > On Mon, May 9, 2022 at 2:35 PM Adrian Fiergolski > wrote: >> Hi Oleksandr, >> >> On 09.05.2022 13:02, Oleksandr Suvorov wrote: >>> Hi Adrian, >>> >>> On Wed, May 4, 2022 at 5:28 PM Adrian Fiergolski >>> wrote: >>>> Hi Oleksandr, >>>> >>>> On 03.05.2022 09:42, Michal Simek wrote: >>>>> On 4/11/22 20:00, Adrian Fiergolski wrote: >>>>>> From: Oleksandr Suvorov >>>>>> >>>>>> Introduce a function which passes an fpga compatible string from >>>>>> FIT images to FPGA drivers. This lets the different implementations >>>>>> decide how to handle it. >>>>>> >>>>>> Some code of Jorge Ramirez-Ortiz is reused. >>>>>> >>>>>> Signed-off-by: Oleksandr Suvorov >>>>>> Tested-by: Ricardo Salveti >>>>>> Co-developed-by: Adrian Fiergolski >>>>>> Signed-off-by: Adrian Fiergolski >>>>>> --- >>>>>> common/spl/spl_fit.c | 6 ++---- >>>>>> drivers/fpga/fpga.c | 23 ++++++++++++++++++++++- >>>>>> include/fpga.h | 4 ++++ >>>>>> 3 files changed, 28 insertions(+), 5 deletions(-) >>>>>> >>>>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c >>>>>> index 1bbf824684..0e3c2a94b6 100644 >>>>>> --- a/common/spl/spl_fit.c >>>>>> +++ b/common/spl/spl_fit.c >>>>>> @@ -588,11 +588,9 @@ static int spl_fit_upload_fpga(struct >>>>>> spl_fit_info *ctx, int node, >>>>>> compatible = fdt_getprop(ctx->fit, node, "compatible", NULL); >>>>>> if (!compatible) >>>>>> warn_deprecated("'fpga' image without 'compatible' property"); >>>>>> - else if (strcmp(compatible, "u-boot,fpga-legacy")) >>>>>> - printf("Ignoring compatible = %s property\n", compatible); >>>>>> - ret = fpga_load(0, (void *)fpga_image->load_addr, >>>>>> fpga_image->size, >>>>>> - BIT_FULL); >>>>>> + ret = fit_fpga_load(0, (void *)fpga_image->load_addr, >>>>>> fpga_image->size, >>>>>> + BIT_FULL, compatible); >>>>>> if (ret) { >>>>>> printf("%s: Cannot load the image to the FPGA\n", __func__); >>>>>> return ret; >>>>>> diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c >>>>>> index 3b0a44b242..a306dd81f9 100644 >>>>>> --- a/drivers/fpga/fpga.c >>>>>> +++ b/drivers/fpga/fpga.c >>>>>> @@ -249,8 +249,23 @@ int fpga_loads(int devnum, const void *buf, >>>>>> size_t size, >>>>>> } >>>>>> #endif >>>>>> +int fit_fpga_load(int devnum, const void *buf, size_t bsize, >>>>>> + bitstream_type bstype, const char *compatible) >>>>>> +{ >>>>>> + fpga_desc *desc = fpga_get_desc(devnum); >>>>> this generates warning which you should fix. >>>>> >>>>> + fpga_desc *desc = fpga_get_desc(devnum); >>>>> + ^~~~~~~~~~~~~ >>>>> w+../drivers/fpga/fpga.c: In function ‘fit_fpga_load’: >>>>> w+../drivers/fpga/fpga.c:255:20: warning: initialization discards >>>>> ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] >>>> As it's your patch, could you address it? >>>> >>>> The 'compatible' field can't be set here, as fpga_get_desc returns >>>> 'const fpga_desc * const'. The descriptors table is populated (fpga_add) >>>> in board_init. At that stage, I am afraid, we don't have access to the >>>> fit image information yet to set a proper 'compatible' (would need to >>>> check the code more carefully). Moreover, IMHO it's not the cleanest way >>>> to change the 'compatible' field after the driver's initialisation. >>> Obviously, the "compatible" attribute belongs to an image, not to an FPGA >>> driver. Unfortunately, I don't see an easy way to stick this attribute to an >>> image in the current architecture. Maybe I missed that way, so I'd appreciate >>> any help with this. >>> >>> Anyway, we could come back to the FPGA driver subsystem later and rework >>> it better and deeper. >>> >>> For now, I'd only fix the warning. Are you ok with this? >> I haven't seen straightforward implementation in the current >> architecture as well. However, it's this series of patches which >> introduces 'compatible', so IMHO, it should be done properly. Michal, >> any ideas/opinions? > I see 2 ways to keep all "compatible"-logic in spl_fit_upload_fpga(): > - by extending each vendor-specific *_load() function with another > parameter, which > will contain a unique type for any supported "compatible" value. > - making up a better-fit structure like "fpga_bitstream", more > specific for fpga bs only > and smaller than spl_image_info, packing there a bs ptr, bs size, bs > type, and bs > "compatible" members. > Both ways require changing the interface for all fpga load()-family functions. Which option from Oleksandr's proposal do you think will be easier to push upstream? Regards, Adrian