From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932829AbcHJDKZ (ORCPT ); Tue, 9 Aug 2016 23:10:25 -0400 Received: from mail-yw0-f195.google.com ([209.85.161.195]:35471 "EHLO mail-yw0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932697AbcHJDKY (ORCPT ); Tue, 9 Aug 2016 23:10:24 -0400 Date: Tue, 9 Aug 2016 23:10:21 -0400 From: Tejun Heo To: Fabien Lahoudere Cc: Csaba Kertesz , "open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers)" , open list Subject: Re: [PATCH 1/1] Add CPU temperature sensor support for i.MX53 Message-ID: <20160810031021.GZ25053@mtj.duckdns.org> References: <1470209756-32705-1-git-send-email-fabien.lahoudere@collabora.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1470209756-32705-1-git-send-email-fabien.lahoudere@collabora.co.uk> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, Fabien. Is $SUBJ right? CPU temperature? On Wed, Aug 03, 2016 at 09:35:56AM +0200, Fabien Lahoudere wrote: > From: Csaba Kertesz > > The original patch was made by Richard Zhu for kernel 2.6.x: > > ENGR00134041-MX53-Add-the-SATA-AHCI-temperature-monitor.patch > > The old source code was migrated to the new kernel 3.x. The concept of > value reading was changed a bit: > > 1. The new 3.x kernel functions (imx_phy_reg_read, imx_phy_reg_write) use > 16 bit registers while the original implementation used 32 bit integers > for this purpose. The description seems pretty dated. > 2. The communication is guarded against infinite loop to give up a certain > register reading after 100000 attempts. This number comes from the > original implementation. A new variable (read_attempt) is introduced to > count the trials. That's a very high number for a retry counter. > +/* SATA AHCI temperature monitor */ > +static ssize_t sata_ahci_current_tmp(struct device *dev, struct device_attribute Abbreviating temperature to tmp probably isn't the best idea. Patch looks good to me except for the above nits; however, I wonder whether it being a one-off sysfs attribute is the right approach. Don't we have subsystems and interfaces for things like temperature readings? Thanks. -- tejun