Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S942278AbcJ0Q4C (ORCPT ); Thu, 27 Oct 2016 12:56:02 -0400 Received: from mail-pf0-f179.google.com ([209.85.192.179]:35022 "EHLO mail-pf0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755463AbcJ0Q4A (ORCPT ); Thu, 27 Oct 2016 12:56:00 -0400 From: Kevin Hilman Cc: Michael Turquette , Sekhar Nori , Rob Herring , Frank Rowand , Mark Rutland , Peter Ujfalusi , Russell King , LKML , arm-soc , linux-drm , linux-devicetree , Jyri Sarha , Tomi Valkeinen , David Airlie , Laurent Pinchart Subject: Re: [PATCH 1/2] ARM: memory: da8xx-ddrctl: new driver Organization: BayLibre References: <1477503355-2600-1-git-send-email-bgolaszewski@baylibre.com> <1477503355-2600-2-git-send-email-bgolaszewski@baylibre.com> 10BTo: Bartosz Golaszewski Date: Thu, 27 Oct 2016 09:55:58 -0700 In-Reply-To: <1477503355-2600-2-git-send-email-bgolaszewski@baylibre.com> (Bartosz Golaszewski's message of "Wed, 26 Oct 2016 19:35:54 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (darwin) MIME-Version: 1.0 Content-Type: text/plain To: unlisted-recipients:; (no To-header on input) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1237 Lines: 50 Bartosz Golaszewski writes: > Create a new driver for the da8xx DDR2/mDDR controller and implement > support for writing to the Peripheral Bus Burst Priority Register. > > Signed-off-by: Bartosz Golaszewski [...] > + for (; setting->name; setting++) { > + knob = da8xx_ddrctl_match_knob(setting); > + if (!knob) { > + dev_warn(dev, > + "no such config option: %s\n", setting->name); > + continue; > + } > + > + if (knob->reg > (res->end - res->start - sizeof(u32))) { Why the "- sizeof(u32)"? Shouldn't this just be resource_size(res)? (c.f. linux/ioport.h) > + dev_warn(dev, > + "register offset of '%s' exceeds mapped memory size\n", > + knob->name); > + continue; > + } > + > + reg = __raw_readl(ddrctl + knob->reg); > + reg &= knob->mask; > + reg |= setting->val << knob->shift; > + > + dev_dbg(dev, "writing 0x%08x to %s\n", reg, setting->name); > + > + __raw_writel(reg, ddrctl + knob->reg); Can you use the normal readl/writel here? Or explain why the raw ones are needed? > + } > + > + return 0; > +} > + Otherwise, looks good to me. With the changes above, feel free to add Reviewed-by: Kevin Hilman Kevin