From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::243; helo=mail-it0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x243.google.com (mail-it0-x243.google.com [IPv6:2607:f8b0:4001:c0b::243]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id F094C210D2137 for ; Fri, 15 Jun 2018 08:56:06 -0700 (PDT) Received: by mail-it0-x243.google.com with SMTP id u4-v6so3335330itg.0 for ; Fri, 15 Jun 2018 08:56:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=S2noRm4Jc/7lxpn5qT0MQytNN53CQYC/vWK1eVqSXk0=; b=c2c9cQRwLm/SGIbyZ2YIkFQI1bDfeSfTh9vV/KxTrSlQOjRC9MLw7mJemtqn2NrZDz JsCHTcTF8mpsS2kpUmg+c2QhxLFHNHbVJrEcIWaxKaNeK3qehCWbsjWH7bPwAgUqPS+F 1Yq3U4Hg5rtNJYc9m6bKKP5aZlkhfeH+/8/0M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=S2noRm4Jc/7lxpn5qT0MQytNN53CQYC/vWK1eVqSXk0=; b=WJ4+aMRImj3s1SLeLdf/iZ6HEEwdZU2TYe5qd6GVAEERx3WWu4Iz3pP/hBLhYMfPHe gz6EKZVKWX1INBPr/VUVqaEWPIsfDGTvt8dbL2Kf+6xlI01QCGeTnZePBi0C12nFDB3y xBRTtfbLKKu6zhckn0iLk3ejilijKqC7dpwGAtRcFiORA0AniAo6XU9KqzE+nW9XycAF lFkaCIXC2aZQnQqfu9dMgd0gJjrMdrUTRkz0UaZe+CBFQi5d0qC4tgnqMXTF3XXYWZLU tfyxy/9qIrrw9KJ9dqnvdwG+mor0wQhWAG2sM7EcngV8eXOMljdXP9cBreOJ9FH3aKwp 3z3Q== X-Gm-Message-State: APt69E1awOdjhHzKZiqXHMjEqFXkbmCkLB9v036wMQ9BlDQq0tjLh1km Rk2Dxnx3Umvr/5KV17bXCkRFCgZLWZF06rGvLLNpGg== X-Google-Smtp-Source: ADUXVKJ3JXT+jVlFPq7+Y6D+dKXyC+Zh2M50c0mn9sVXaM03JomcVqCTWFzMj4Tg80bvxdlF4vusD51+0QZ+HDW04oA= X-Received: by 2002:a02:6001:: with SMTP id i1-v6mr1363383jac.5.1529078165675; Fri, 15 Jun 2018 08:56:05 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bbc7:0:0:0:0:0 with HTTP; Fri, 15 Jun 2018 08:56:05 -0700 (PDT) In-Reply-To: References: <20180615141348.13020-1-sami.mujawar@arm.com> From: Ard Biesheuvel Date: Fri, 15 Jun 2018 17:56:05 +0200 Message-ID: To: Evan Lloyd Cc: Sami Mujawar , "edk2-devel@lists.01.org" , "Zeng, Star" , Eric Dong , Ruiyu Ni , Leif Lindholm , Matteo Carlini , Stephanie Hughes-Fitt , Thomas Abraham , nd Subject: Re: [PATCH v2] MdeModulePkg: Enable SATA Controller PCI mem space X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Jun 2018 15:56:07 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On 15 June 2018 at 17:26, Evan Lloyd wrote: > Hi Ard, Zeng > >> -----Original Message----- >> From: Ard Biesheuvel >> Sent: 15 June 2018 15:17 >> To: Sami Mujawar >> Cc: edk2-devel@lists.01.org; Zeng, Star ; Eric Dong >> ; Ruiyu Ni ; Leif Lindholm >> ; Matteo Carlini ; >> Stephanie Hughes-Fitt ; Evan Lloyd >> ; Thomas Abraham ; >> nd >> Subject: Re: [PATCH v2] MdeModulePkg: Enable SATA Controller PCI mem >> space >> >> On 15 June 2018 at 16:13, Sami Mujawar wrote: >> > The SATA controller driver crashes while accessing the PCI memory >> > [AHCI Base Registers (ABAR)], as the PCI memory space is not enabled. >> > >> > Enable the PCI memory space access to prevent the SATA Controller >> > driver from crashing. >> > >> > Contributed-under: TianoCore Contribution Agreement 1.1 >> > Signed-off-by: Sami Mujawar >> > --- >> > The changes can be viewed at >> > >> https://github.com/samimujawar/edk2/tree/284_sata_controler_pci_mem_f >> i >> > x_v2 >> > >> > Notes: >> > v2: >> > - Improved log message and code documentation based on feedback >> [SAMI] >> > - Enable IO space, suggestion to use EFI_PCI_DEVICE_ENABLE = [ZENG] >> > - This SATA Controller driver only uses the PCI BAR5 register >> > space which is the AHCI Base Address (ABAR). According to the >> > 'Serial ATA Advanced Host Controller Interface (AHCI) 1.3.1' >> > specification, section 2.1.11, 'This register allocates space >> > for the HBA memory registers'. >> > The section 2.1.10, allows provision for Optional BARs which >> > may support either memory or I/O spaces. However, in the context >> > of the current SATA controller driver, which only ever access >> > the ABAR, enabling I/O memory space is not required. = [SAMI] >> > - Prefer to use // surrounding comments = [ZENG] >> > - Doing this would violate the edk2 coding standard. See EDK2 >> > Coding Standard Specification, revision 2.20, section 6.2.3. = [SAMI] >> > >> >> Please stop obsessing about the coding standard. The maintainer was quit= e >> clear what he wanted, and in the past, I have also indicated that for th= e ARM >> related packages, I prefer readability and consistency over adherence to= a >> dubious standard. > > [[Evan Lloyd]] I'd like to make some points here: > 1. It is perfectly reasonable for Sami to explain why he has done so= mething a certain way - Zeng is then at liberty to respond with his prefere= nce, but we do not (yet) know what that might be. > Yes, we do. He already told us his preference. > 2. "readability and consistency" is the very purpose of any coding s= tandard. If consistency is valuable, doesn't that apply across modules? I= understand that it may be particularly valuable for maintainers within the= ir modules, but the rest of us benefit from a consistent style - especially= when looking outside our normal demesne. > > 3. Applying a consistent Coding Standard across the whole project is= a necessary pre-condition for automated CI checks on new submissions. I'd= like to aim e.g. for an improved patchcheck.py, but that requires consiste= ncy across modules, at least for new code. > > Note: I am not disputing the dubiosity of the CCS. It could be improved= (a lot). However it is all we have right now. > OK, so you are arguing that we should adhere to the letter of the CCS even though there is consensus about its shortcomings and its detachment from reality? I think that is a waste of everybody's time.