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 X-Spam-Level: X-Spam-Status: No, score=-0.7 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 75C8EC433E0 for ; Sun, 14 Mar 2021 10:41:20 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D4EE864E27 for ; Sun, 14 Mar 2021 10:41:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D4EE864E27 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=MdapysxPmOcAKnamazd7HPf1S8FGgpv1W+vFkYEcim8=; b=NPF0rrtFw4lawAyi+ZnsFcvZC 0o5pZTcESlZCRSpWKkMswavlzy5iGO8Jev41dJCy89zR4u0GrGkieR8DAkTXh/ikyp47ScOKIrRaa 1NOSs1YIv27PjNp4gTocOeSWn77ixfssDIc1kmLs/sDt1vZ5cOHP9f5qUDmlexGmizkTgh6hqsRqQ H8XjbABBjKpTrTGjrYp00SKl5BBd89qy0QsnkBvOlsLJRR36Qm3Z4EM+2bhL5zyO9IGjMaiYNUirl d+plov0aOkjdhB+hUcSIGQz6425cTBnuSwjGqds6pgileAX3nwzu6uYTbd2lSaSe0KGeiDJL9i0bZ gDoIpQHTg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lLO9m-00E7cx-Gp; Sun, 14 Mar 2021 10:39:34 +0000 Received: from mail-ej1-x62a.google.com ([2a00:1450:4864:20::62a]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lLO9h-00E7cU-Ev; Sun, 14 Mar 2021 10:39:31 +0000 Received: by mail-ej1-x62a.google.com with SMTP id r17so61426403ejy.13; Sun, 14 Mar 2021 03:39:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=jjsmnr5FzYMoxt2sTlQ7jPd6uzhNAuqUavfnO0MllWI=; b=C3Ps+AyEjo30X/ua5dUcFTMc2/1wzyGikyEVOYts0yq88Nvc97piiqc7rkqUAMHu1S LMsVPcQSuwEnaZ+2S6uzQfrUlfRUH9HFoKUcAhsOkjvWnr81VMvvYlcXSNK1Xfo3KHGa e4oL0YgYI5YAkCUYVdWv4ba2Ltm06Jrf4IC0mvTNEX0mN/i2UNB/d2LgqbOk8P3P3Nw1 chQOawVQiix8IkLgJPffD+UMfgzBjWxb4HrLjHqCTHZmgmWsEExuEqfCy6X5TLZKfvk7 JPpnolbvwWljKmTvMIVDWM1aFgzs5iU4S2SCu7NuHvIZYRLyFs8zHPXLi5vqhc9asM1k +o7Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=jjsmnr5FzYMoxt2sTlQ7jPd6uzhNAuqUavfnO0MllWI=; b=qm56RfLOWEQOMxQyk2e65G4GFpbOoN6OaHPXKMG2A0pfduICrn+PErZDSWOU630gbH PK8emDRJX+YbqM0IPmA62dmpoZNFXe4MWjM+D94SLJ4ds+AfUnw87Ac5BJlliEF+zD5+ CUCyiqAblZe+Z0pcSMUae/C51W+51gOwE0n3cFTD++HXfGhuOME0RAnAPFiVWosznOZK ZewHK5DxziVY0a04LT7dCK5oHo23xbPaxjwPGntwz+ywop9xQxX0GgZYybwvLaR4h2bn 4YC6rVKE5PdtlTXy5t0Z2yYKtx/j2A6Ib5MeoFDNonoU8LSFCObiTUO8h3G3nFket6pU Ce/Q== X-Gm-Message-State: AOAM533yAjG4NZSlcW16C0gS0iL3SjK8AhfB+6E4oN9GvLRPuQEHpZ6e 32xG5PwG/sS83dG8JS3jxzo= X-Google-Smtp-Source: ABdhPJxWgxN4PoB/cokJVbMonJFcjsPd6jis+Pn3rsvukjKRHYFSjOpz2IvaNc8qtfEJTj96MWWjhg== X-Received: by 2002:a17:906:110d:: with SMTP id h13mr18343476eja.357.1615718366915; Sun, 14 Mar 2021 03:39:26 -0700 (PDT) Received: from ubuntu2004 ([188.24.140.160]) by smtp.gmail.com with ESMTPSA id cw14sm6219111edb.8.2021.03.14.03.39.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 14 Mar 2021 03:39:25 -0700 (PDT) Date: Sun, 14 Mar 2021 12:39:26 +0200 From: Cristian Ciocaltea To: Andrew Lunn Cc: "David S. Miller" , Jakub Kicinski , Rob Herring , Andreas =?iso-8859-1?Q?F=E4rber?= , Manivannan Sadhasivam , Philipp Zabel , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-actions@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] net: ethernet: actions: Add Actions Semi Owl Ethernet MAC driver Message-ID: <20210314103926.GA418860@ubuntu2004> References: <158d63db7d17d87b01f723433e0ddc1fa24377a8.1615423279.git.cristian.ciocaltea@gmail.com> <20210314011324.GA991090@BV030612LT> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210314_103929_814204_0F248B0D X-CRM114-Status: GOOD ( 41.60 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, Mar 14, 2021 at 05:36:32AM +0100, Andrew Lunn wrote: > > > > + if (phy->interface != PHY_INTERFACE_MODE_RMII) { > > > > + netdev_err(netdev, "unsupported phy mode: %s\n", > > > > + phy_modes(phy->interface)); > > > > + phy_disconnect(phy); > > > > + netdev->phydev = NULL; > > > > + return -EINVAL; > > > > + } > > > > > > It looks like the MAC only supports symmetric pause. So you should > > > call phy_set_sym_pause() to let the PHY know this. > > > > I did not find any reference related to the supported pause types, > > is this normally dependant on the PHY interface mode? > > There is a MAC / PHY split there. The PHY is responsible for the > negotiation for what each end can do. But it is the MAC which actually > implements pause. The MAC needs to listen to pause frames and not send > out data frames when the link peer indicates pause. And the MAC needs > to send a pause frames when its receive buffers are full. The code you > have in this MAC driver seems to indicate the MAC only supports > symmetric pause. Hence you need to configure the PHY to only auto-neg > symmetric pause. Thanks for explaining this, I will implement the indicated PHY configuration and, additionally, also enable the SMII interface. > > > > + ret = crypto_skcipher_encrypt(req); > > > > + if (ret) { > > > > + dev_err(dev, "failed to encrypt S/N: %d\n", ret); > > > > + goto err_free_tfm; > > > > + } > > > > + > > > > + netdev->dev_addr[0] = 0xF4; > > > > + netdev->dev_addr[1] = 0x4E; > > > > + netdev->dev_addr[2] = 0xFD; > > > > > > 0xF4 has the locally administered bit 0. So this is a true OUI. Who > > > does it belong to? Ah! > > > > > > F4:4E:FD Actions Semiconductor Co.,Ltd.(Cayman Islands) > > > > > > Which makes sense. But is there any sort of agreement this is allowed? > > > It is going to cause problems if they are giving out these MAC > > > addresses in a controlled way. > > > > Unfortunately this is another undocumented logic taken from the vendor > > code. I have already disabled it from being built by default, although, > > personally, I prefer to have it enabled in order to get a stable MAC > > address instead of using a randomly generated one or manually providing > > it via DT. > > > > Just for clarification, I did not have any agreement or preliminary > > discussion with the vendor. This is just a personal initiative to > > improve the Owl SoC support in the mainline kernel. > > > > > Maybe it would be better to set bit 1 of byte 0? And then you can use > > > 5 bytes from enc_sn, not just 4. > > > > I included the MAC generation feature in the driver to be fully > > compatible with the original implementation, but I'm open for changes > > if it raises concerns and compatibility is less important. > > This is not a simple question to answer. If the vendor driver does > this, then the vendor can never assign MAC addresses in a controlled > way, unless they have a good idea how the algorithm turns serial > numbers into MAC addresses, and they can avoid MAC addresses for > serial numbers already issued. > > But should the Linux kernel do the same? If all you want is a stable > MAC address, my personal preference would be to set the locally > administered bit, and fill the other 5 bytes from the hash > algorithm. You then have a stable MAC addresses, but you clearly > indicate it is not guaranteed to by globally unique, and you do not > need to worry about what the vendor is doing. I fully agree, so I'm going to set byte 0 to value 0xF6 and replace bytes 1 & 2 with entries from the computed hash. I will also document this modification and the rationale behind. > > > Otherwise, this look a new clean driver. > > > > Well, I tried to do my best, given my limited experience as a self-taught > > kernel developer. Hopefully reviewing my code will not cause too many > > headaches! :) > > This is actually above average for a self-taught kernel > developer. Well done. Thank you, Andrew! > Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel