Re: [PATCH v4 01/16] drm: exynos/dp: fix code style

2015-09-02 Thread Joe Perches 
On Thu, 2015-09-03 at 13:33 +0800, Yakir Yang wrote:
[]
>  diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c
[]
>  @@ -155,24 +156,22 @@ static int exynos_dp_read_edid(struct
>  exynos_dp_device *dp)
> }
>   exynos_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
>  -_vector);
>  +  _vector);
> if (test_vector & DP_TEST_LINK_EDID_READ) {
>  -exynos_dp_write_byte_to_dpcd(dp,
>  -DP_TEST_EDID_CHECKSUM,
>  +exynos_dp_write_byte_to_dpcd(
>  +dp, DP_TEST_EDID_CHECKSUM,
> edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]);
>  -exynos_dp_write_byte_to_dpcd(dp,
>  -DP_TEST_RESPONSE,
>  +exynos_dp_write_byte_to_dpcd(
>  +dp, DP_TEST_RESPONSE,
> DP_TEST_EDID_CHECKSUM_WRITE);
> >>> To me, missing argument after opening parenthesis, looks worse. I would
> >>> prefer:
> >>>
> >>>  exynos_dp_write_byte_to_dpcd(dp,
> >>>
> >>> Why you moved the 'dp' argument to new line?
> >> Hmm... Just like style tool indicate, no more warning after
> >> that change.
> >>
> >> For now, I would like to follow the original style, just improved
> >> some obvious style problem.  :-)
> > What was the checkpatch warning that said 'dp' has to move to new line?
> > I tried this and I don't see it.
> 
> checkpatch haven't remind me that put dp to new line would fix
> this warning, this just come from my experiments. And I works,
> no more warnings from checkpatch, so I toke this style.

Checkpatch isn't a great arbiter of style.
It's just a brainless tool.

Always use your instead of anything brainless.

If it were code I was writing, I'd ignore 80 columns warnings
where appropriate.

These are long function names and long macro defines, so it's
inappropriate to use 80 columns as a guiding style.

I'd write:

exynos_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST, 
_vector);
if (test_vector & DP_TEST_LINK_EDID_READ) {
exynos_dp_write_byte_to_dpcd(dp, DP_TEST_EDID_CHECKSUM,
 edid[EDID_BLOCK_LENGTH + 
EDID_CHECKSUM]);
exynos_dp_write_byte_to_dpcd(dp, DP_TEST_RESPONSE,
 
DP_TEST_EDID_CHECKSUM_WRITE);
}


--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 1/8] drm: exynos/dp: fix code style

2015-08-06 Thread Joe Perches 
On Thu, 2015-08-06 at 09:04 -0500, Yakir Yang wrote:
 make checkpatch.pl script happy

That should not be the primary reason to submit a patch.

Making it easier for human code reader to understand
what the code does should be though.

 diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c 
 b/drivers/gpu/drm/exynos/exynos_dp_core.c
[]
 @@ -123,10 +123,11 @@ static int exynos_dp_read_edid(struct exynos_dp_device 
 *dp)
   dev_dbg(dp-dev, EDID data includes a single extension!\n);
  
   /* Read EDID data */
 - retval = exynos_dp_read_bytes_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
 - EDID_HEADER_PATTERN,
 - EDID_BLOCK_LENGTH,
 - edid[EDID_HEADER_PATTERN]);
 + retval =
 + exynos_dp_read_bytes_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
 +   EDID_HEADER_PATTERN,
 +   EDID_BLOCK_LENGTH,
 +   edid[EDID_HEADER_PATTERN]);

This is a relatively uncommon style.

Because the code uses relatively long variable and
function names as well as longish macro #defines,
prefer to ignore the 80 column limit.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/20] fix misspelling of current function in string

2014-12-07 Thread Joe Perches 
On Sun, 2014-12-07 at 20:20 +0100, Julia Lawall wrote:
 These patches replace what appears to be a reference to the name of the
 current function but is misspelled in some way by either the name of the
 function itself, or by %s and then __func__ in an argument list.

At least a few of these seem to be function tracing
style uses that might as well be deleted instead.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday

2014-09-19 Thread Joe Perches 
On Fri, 2014-09-19 at 12:26 +0200, Javier Martinez Canillas wrote:
 The function max77686_rtc_calculate_wday() is used to
 calculate the day of the week to be filled in struct
 rtc_time but that function only calculates the number
 of bits shifted. So the ffs() function can be used to
 find the first bit set instead of a special function.

This isn't the same logic.  Perhaps you want fls.

 diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
[]
 -static inline int max77686_rtc_calculate_wday(u8 shifted)
 -{
 - int counter = -1;
 - while (shifted) {
 - shifted = 1;
 - counter++;
 - }
 - return counter;
 -}
 -
  static void max77686_rtc_data_to_tm(u8 *data, struct rtc_time *tm,
  int rtc_24hr_mode)
  {
 @@ -93,7 +83,7 @@ static void max77686_rtc_data_to_tm(u8 *data, struct 
 rtc_time *tm,
   tm-tm_hour += 12;
   }
  
 - tm-tm_wday = max77686_rtc_calculate_wday(data[RTC_WEEKDAY]  0x7f);
 + tm-tm_wday = ffs(data[RTC_WEEKDAY]  0x7f) - 1;



--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v10 5/6] rtc: max77686: Use ffs() to calculate tm_wday

2014-09-19 Thread Joe Perches 
On Fri, 2014-09-19 at 21:27 +0200, Javier Martinez Canillas wrote:
 Hello Joe,

Hello Javier.

 On 09/19/2014 04:39 PM, Joe Perches wrote:
  On Fri, 2014-09-19 at 12:26 +0200, Javier Martinez Canillas wrote:
  The function max77686_rtc_calculate_wday() is used to
  calculate the day of the week to be filled in struct
  rtc_time but that function only calculates the number
  of bits shifted. So the ffs() function can be used to
  find the first bit set instead of a special function.
  
  This isn't the same logic.  Perhaps you want fls.
 
 
 Right, the removed function has the same logic than fls() - 1 but the value
 stored in data[RTC_WEEKDAY] is:
 
 data[RTC_WEEKDAY] = 1  tm-tm_wday;
 
 so for this particular case, it doesn't matter since ffs() == fls() always.

I didn't look that far.

I just wanted to show that the logic wasn't the same.

Perhaps you can specify that ffs==fls in the changelog.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] serial: samsung: fix typo in debug code

2014-06-04 Thread Joe Perches 
On Tue, 2014-06-03 at 11:53 +0200, Arnd Bergmann wrote:
 commit e4ac92df2791 (serial: samsung: Neaten dbg uses) introduced
 a regression in the conversion from vsprintf to vsnprintf.
 
 This fixes the build error by passing the correct variable name.
 
 Signed-off-by: Arnd Bergmann a...@arndb.de
 Cc: Joe Perches j...@perches.com
 Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
 Cc: Doug Anderson diand...@chromium.org
 Cc: Kukjin Kim kgene@samsung.com
 
 diff --git a/drivers/tty/serial/samsung.c b/drivers/tty/serial/samsung.c
 index 3293377..c1d3ebd 100644
 --- a/drivers/tty/serial/samsung.c
 +++ b/drivers/tty/serial/samsung.c
 @@ -66,7 +66,7 @@ static void dbg(const char *fmt, ...)
   char buff[256];
  
   va_start(va, fmt);
 - vscnprintf(buff, sizeof(buf), fmt, va);
 + vscnprintf(buff, sizeof(buff), fmt, va);
   va_end(va);
  
   printascii(buff);

Greg?

The original has ended up in Linus' -next branch.

Can you please apply this sooner rather than later?

Sorry for the bother.  Dunno how it happened.

I thought I compiled it with the appropriate CONFIG
settings.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V11 2/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver

2014-03-22 Thread Joe Perches 
On Sat, 2014-03-22 at 14:01 -0500, Vince Bridgers wrote:
 See comments inline

Do please trim your comments instead of quoting the
_whole_ email.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V4 0/8] add new Samsung SXGBE driver

2014-03-18 Thread Joe Perches 
On Mon, 2014-03-17 at 23:47 -0700, Byungho An wrote:
 Hi all,

Hello all.

  drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c| 2382
 

The patches are wrapped again and can not be applied.

It really would be more convenient if you could use
git send-email for these.

Please resend.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V5 2/8] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver

2014-03-18 Thread Joe Perches 
On Tue, 2014-03-18 at 11:19 -0700, Byungho An wrote:
 From: Siva Reddy siva.kal...@samsung.com
 
 This patch adds support for Samsung 10Gb ethernet driver(sxgbe).

More trivia, nothing that should stop this from
being applied and updated later...

 diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_mdio.c 
 b/drivers/net/ethernet/samsung/sxgbe/sxgbe_mdio.c
[]
 +static int sxgbe_mdio_busy_wait(void __iomem *ioaddr, unsigned int mii_data)
 +{
 + unsigned long cur_time;
 + unsigned long fin_time = jiffies + 3 * HZ; /* 30 ms */

This is actually 3 seconds.
If this should be 30ms, then use msecs_to_jiffies(30)

 +
 + do {
 + cur_time = jiffies;
 + if (readl(ioaddr + mii_data)  SXGBE_MII_BUSY)
 + cpu_relax();
 + else
 + return 0;
 + } while (!time_after_eq(cur_time, fin_time));
 +
 + return -EBUSY;
 +}

This may be clearer as

unsigned long fin_time = jiffies + msecs_to_jiffies(30);
 
while (!time_after(jiffies, fin_time))
if (!(readl(ioaddr + mii_data)  SXGBE_MII_BUSY))
return 0;
cpu_relax();
}

return -EBUSY;


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V3 2/8] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver

2014-03-17 Thread Joe Perches 
On Mon, 2014-03-17 at 13:43 -0700, Byungho An wrote:
 From: Siva Reddy siva.kal...@samsung.com
 
 This patch adds support for Samsung 10Gb ethernet driver(sxgbe).

Sorry about the brevity, but this driver is very long and
I don't like to read that much at a time, so this is very
superficial and I didn't read the whole thing.

Trivial notes:

 diff --git a/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c 
 b/drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c

[]

 +static void sxgbe_clk_csr_set(struct sxgbe_priv_data *priv)
 +{
 + u32 clk_rate = clk_get_rate(priv-sxgbe_clk);
 +
 + /* assign the proper divider, this will be used during
 +  * mdio communication
 +  */
 + if (clk_rate  SXGBE_CSR_F_150M)
 + priv-clk_csr = SXGBE_CSR_100_150M;
 + else if ((clk_rate = SXGBE_CSR_F_150M) 
 +  (clk_rate = SXGBE_CSR_F_250M))
 + priv-clk_csr = SXGBE_CSR_150_250M;
 + else if ((clk_rate = SXGBE_CSR_F_250M) 
 +  (clk_rate = SXGBE_CSR_F_300M))
 + priv-clk_csr = SXGBE_CSR_250_300M;
 + else if ((clk_rate = SXGBE_CSR_F_300M) 
 +  (clk_rate = SXGBE_CSR_F_350M))
 + priv-clk_csr = SXGBE_CSR_300_350M;
 + else if ((clk_rate = SXGBE_CSR_F_350M) 
 +  (clk_rate = SXGBE_CSR_F_400M))
 + priv-clk_csr = SXGBE_CSR_350_400M;
 + else if ((clk_rate = SXGBE_CSR_F_400M) 
 +  (clk_rate = SXGBE_CSR_F_500M))
 + priv-clk_csr = SXGBE_CSR_400_500M;
 +}

This block is unnecessarily hard to read and would be
better without the superfluous tests as:

if (clk_rate  SXGBE_CSR_F_150M)
priv-clk_csr = SXGBE_CSR_100_150M;
else if (clk_rate = SXGBE_CSR_F_250M)
priv-clk_csr = SXGBE_CSR_150_250M;
else if (clk_rate = SXGBE_CSR_F_300M)
priv-clk_csr = SXGBE_CSR_250_300M;
else if (clk_rate = SXGBE_CSR_F_350M)
priv-clk_csr = SXGBE_CSR_300_350M;
else if (clk_rate = SXGBE_CSR_F_400M)
priv-clk_csr = SXGBE_CSR_350_400M;
else if (clk_rate = SXGBE_CSR_F_500M)
priv-clk_csr = SXGBE_CSR_400_500M;

I don't understand why the first test is  and the
subsequent tests are = though.

 +static int init_tx_ring(struct device *dev, u8 queue_no,
 + struct sxgbe_tx_queue *tx_ring, int tx_rsize)
 +{
[]
 + /* allocate memory for TX descriptors */
 + tx_ring-dma_tx = dma_zalloc_coherent(dev,
 +   tx_rsize * sizeof(struct 
 sxgbe_tx_norm_desc),
 +   tx_ring-dma_tx_phy, GFP_KERNEL);
 + if (!tx_ring-dma_tx) {
 + dev_err(dev, No memory for TX desc of SXGBE\n);
 + return -ENOMEM;
 + }
 +
 + /* allocate memory for TX skbuff array */
 + tx_ring-tx_skbuff_dma = devm_kcalloc(dev, tx_rsize,
 +   sizeof(dma_addr_t), GFP_KERNEL);
 + if (!tx_ring-tx_skbuff_dma) {
 + dev_err(dev, No memory for TX skbuffs DMA of SXGBE\n);
 + goto dmamem_err;
 + }
 +
 + tx_ring-tx_skbuff = devm_kcalloc(dev, tx_rsize,
 +   sizeof(struct sk_buff *), GFP_KERNEL);
 +
 + if (!tx_ring-tx_skbuff) {
 + dev_err(dev, No memory for TX skbuffs of SXGBE\n);
 + goto dmamem_err;
 + }

All the OOM messages like these are unnecessary as there is an
existing generic OOM on allocation failures and a dump_stack()

[]

 +static netdev_tx_t sxgbe_xmit(struct sk_buff *skb, struct net_device *dev)
 +{
[]
 + /* display current ring */
 + if (netif_msg_pktdata(priv)) {
 + netdev_dbg(dev, %s: curr %d dirty=%d entry=%d, first=%p, 
 nfrags=%d\n,
 +__func__, (tqueue-cur_tx % tx_rsize),
 +(tqueue-dirty_tx % tx_rsize), entry,
 +first_desc, nr_frags);

These 2 lines can be combined (and unnecessary parentheses removed)
into:

netif_dbg(priv, pktdata, dev, %s: curr %d dirty=%d entry=%d, first=%p, 
nfrags=%d\n,
  __func__, tqueue-cur_tx % tx_rsize,
  tqueue-dirty_tx % tx_rsize, entry,
  first_desc, nr_frags);

[]

 +/*  sxgbe_get_stats64 - entry point to see statistical information of device
 + *  @dev : device pointer.
 + *  @stats : pointer to hold all the statistical information of device.
 + *  Description:
 + *  This function is a driver entry point whenever ifconfig command gets
 + *  executed to see device statistics. Statistics are number of
 + *  bytes sent or received, errors occured etc.
 + *  Return value:
 + *  This function returns various statistical information of device.
 + */
 +static struct rtnl_link_stats64 *sxgbe_get_stats64(struct net_device *dev,
 +struct rtnl_link_stats64 
 *stats)
 +{
 + struct sxgbe_priv_data *priv = netdev_priv(dev);
 +

Re: [PATCH V2 RE-SEND 1/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver

2014-03-13 Thread Joe Perches 
On Thu, 2014-03-13 at 15:55 +0900, Byungho An wrote:
 This patch adds support for Samsung 10Gb ethernet driver(sxgbe).
 - sxgbe core initialization
 - Tx and Rx support
 - MDIO support
 - ISRs for Tx and Rx
 - ifconfig support to driver
[]
 diff --git a/drivers/net/ethernet/samsung/sxgbe_desc.c 
 b/drivers/net/ethernet/samsung/sxgbe_desc.c
[]
 +/* Set Time stamp */
 +static void sxgbe_tx_ctxt_desc_set_tstamp(struct sxgbe_tx_ctxt_desc *p,
 +   u8 ostc_enable, u64 tstamp)
 +{
 + if (ostc_enable) {
 + p-ostc = ostc_enable;
 + p-tstamp_lo = (u32) tstamp;
 + p-tstamp_hi = (u32) (tstamp32);
 + }
 +}

[]

 +static u64 sxgbe_get_rx_timestamp(struct sxgbe_rx_ctxt_desc *p)
 +{
 + u64 ns;
 + ns = p-tstamp_lo;
 + ns += p-tstamp_hi * 10ULL;
 +
 + return ns;
 +}

This looks odd.

rx and tx timestamps are different clocks?

rx tstamp_lo resets every second but tx_stamp
is free-running?

Maybe this was supposed to be something like

ns = p-tstamp_lo
ns |= ((u64)tstamp_hi)  32;

If not, maybe it warrants a comment around
here or on the descriptor definition

 diff --git a/drivers/net/ethernet/samsung/sxgbe_desc.h 
 b/drivers/net/ethernet/samsung/sxgbe_desc.h
[]
 +/* Context descriptor structure */
 +struct sxgbe_tx_ctxt_desc {
 + u32 tstamp_lo:32;
 + u32 tstamp_hi:32;

[]

 +struct sxgbe_rx_ctxt_desc {
 + u32 tstamp_lo:32;
 + u32 tstamp_hi:32;
 


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 RE-SEND 1/7] net: sxgbe: add basic framework for Samsung 10Gb ethernet driver

2014-03-13 Thread Joe Perches 
On Thu, 2014-03-13 at 05:53 -0700, Joe Perches wrote:
 Maybe this was supposed to be something like
 
   ns = p-tstamp_lo
   ns |= ((u64)tstamp_hi)  32;
 
 If not, maybe it warrants a comment around
 here or on the descriptor definition
[]
  +struct sxgbe_rx_ctxt_desc {
  +   u32 tstamp_lo:32;
  +   u32 tstamp_hi:32;

or maybe these would be better renamed like:

u32 tstamp_nsecs;
u32 tstamp_secs;

btw2: I still think using

u32 foo:32;

is asking for gcc to generate bad code.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 0/7] add new Samsung sxgbe driver

2014-03-12 Thread Joe Perches 
On Wed, 2014-03-12 at 22:27 +0900, Byungho An wrote:
 Hi all,
[]
  drivers/net/ethernet/samsung/sxgbe_main.c  | 2447
 

The patch set is word wrapped and can not be applied.

Please resend without the word wrapping.

Using Outlook can make this difficult.

If possible, use git send-email.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] net: xgmac: add basic framework for Samsung 10Gb ethernet driver

2014-03-05 Thread Joe Perches 
On Wed, 2014-03-05 at 20:28 +0900, Byungho An wrote:
 From: Siva Reddy siva.kal...@samsung.com

Just a few trivial comments on a brief scan.

 +/* Context descriptor structure */
 +struct xgmac_tx_ctxt_desc {
 + u32 tstamp_lo:32;
 + u32 tstamp_hi:32;

I think u32 foo:32; is odd at best
and may cause compiler oddities.

 diff --git a/drivers/net/ethernet/samsung/xgmac_main.c 
 b/drivers/net/ethernet/samsung/xgmac_main.c
[]
 +static void print_pkt(unsigned char *buf, int len)
 +{
 + int j;
 + pr_debug(len = %d byte, buf addr: 0x%p, len, buf);
 + for (j = 0; j  len; j++) {
 + if ((j % 16) == 0)
 + pr_debug(\n %03x:, j);
 + pr_debug( %02x, buf[j]);
 + }
 + pr_debug(\n);
 +}

This will make a mess.  Use print_hex_dump_debug.

 +static int xgmac_open(struct net_device *dev)
 +{
[]
   if (ret) {
 + pr_err(%s: Cannot attach to PHY (error: %d)\n,
 + __func__, ret);

A lot of the message logging would be better using
more verbose forms like;

netdev_level(dev, etc...)

[]

 +/* xgmac_config - entry point for changing configuration mode passed on by
 + * ifconfig
 + * @dev : pointer to the device structure
 + * @map : pointer to the device mapping structure
 + * Description:
 + * This function is a driver entry point which gets called by the kernel
 + * whenever some device configuration is changed.
 + * Return value:
 + * This function returns 0 if success and appropriate error otherwise.
 + */

It might be nicer if you used kernel-doc forms startig with
/**
for these comments everywhere.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] samsung: xgmac: Neatening

2014-03-05 Thread Joe Perches 
Quiet checkpatch noise:
o Multi-line statement alignment
o Add braces
o Move logical continuations
o Remove externs from .h
o Remove unnecessary blank lines around braces

Typo fixes where noticed.
Change logic to return first to reduce indentation.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/net/ethernet/samsung/xgmac_common.h  | 24 
 drivers/net/ethernet/samsung/xgmac_desc.c|  4 --
 drivers/net/ethernet/samsung/xgmac_dma.c | 32 +-
 drivers/net/ethernet/samsung/xgmac_ethtool.c | 67 ++--
 drivers/net/ethernet/samsung/xgmac_main.c| 92 +---
 drivers/net/ethernet/samsung/xgmac_mdio.c| 13 ++--
 drivers/net/ethernet/samsung/xgmac_mtl.c |  4 +-
 7 files changed, 115 insertions(+), 121 deletions(-)

diff --git a/drivers/net/ethernet/samsung/xgmac_common.h 
b/drivers/net/ethernet/samsung/xgmac_common.h
index 4932824..4c46504 100644
--- a/drivers/net/ethernet/samsung/xgmac_common.h
+++ b/drivers/net/ethernet/samsung/xgmac_common.h
@@ -544,24 +544,24 @@ struct xgmac_priv_data {
 };
 
 /* Function prototypes */
-extern struct xgmac_priv_data *xgmac_dvr_probe(struct device *device,
+struct xgmac_priv_data *xgmac_dvr_probe(struct device *device,
struct xgmac_plat_data *plat_dat,
void __iomem *addr);
-extern int xgmac_dvr_remove(struct net_device *ndev);
-extern void xgmac_set_ethtool_ops(struct net_device *netdev);
-extern int xgmac_mdio_unregister(struct net_device *ndev);
-extern int xgmac_mdio_register(struct net_device *ndev);
-extern int xgmac_register_platform(void);
-extern void xgmac_unregister_platform(void);
+int xgmac_dvr_remove(struct net_device *ndev);
+void xgmac_set_ethtool_ops(struct net_device *netdev);
+int xgmac_mdio_unregister(struct net_device *ndev);
+int xgmac_mdio_register(struct net_device *ndev);
+int xgmac_register_platform(void);
+void xgmac_unregister_platform(void);
 
 #ifdef CONFIG_PM
-extern int xgmac_suspend(struct net_device *ndev);
-extern int xgmac_resume(struct net_device *ndev);
-extern int xgmac_freeze(struct net_device *ndev);
-extern int xgmac_restore(struct net_device *ndev);
+int xgmac_suspend(struct net_device *ndev);
+int xgmac_resume(struct net_device *ndev);
+int xgmac_freeze(struct net_device *ndev);
+int xgmac_restore(struct net_device *ndev);
 #endif /* CONFIG_PM */
 
-extern const struct xgmac_mtl_ops *xgmac_get_mtl_ops(void);
+const struct xgmac_mtl_ops *xgmac_get_mtl_ops(void);
 
 void xgmac_disable_eee_mode(struct xgmac_priv_data * const priv);
 bool xgmac_eee_init(struct xgmac_priv_data * const priv);
diff --git a/drivers/net/ethernet/samsung/xgmac_desc.c 
b/drivers/net/ethernet/samsung/xgmac_desc.c
index c5417de..ddf3e94 100644
--- a/drivers/net/ethernet/samsung/xgmac_desc.c
+++ b/drivers/net/ethernet/samsung/xgmac_desc.c
@@ -196,7 +196,6 @@ static void xgmac_tx_ctxt_desc_set_tstamp(struct 
xgmac_tx_ctxt_desc *p,
p-tstamp_lo = (u32) tstamp;
p-tstamp_hi = (u32) (tstamp32);
}
-
 }
 /* Close TX context descriptor */
 static void xgmac_tx_ctxt_desc_close(struct xgmac_tx_ctxt_desc *p)
@@ -217,7 +216,6 @@ static void xgmac_init_rx_desc(struct xgmac_rx_norm_desc 
*p, int disable_rx_ic,
p-rdes23.rx_rd_des23.own_bit = 1;
if (disable_rx_ic)
p-rdes23.rx_rd_des23.int_on_com = disable_rx_ic;
-
 }
 
 /* Get RX own bit */
@@ -337,7 +335,6 @@ static int xgmac_rx_wbstatus(struct xgmac_rx_norm_desc *p,
pr_err(\tInvalid L2 Packet type\n);
break;
}
-
}
 
/* L3/L4 Pkt type */
@@ -443,7 +440,6 @@ static void xgmac_rx_ctxt_wbstatus(struct 
xgmac_rx_ctxt_desc *p,
x-rx_ptp_signal++;
else if (p-ptp_msgtype == RX_PTP_RESV_MSG)
x-rx_ptp_resv_msg_type++;
-
 }
 
 /* get rx timestamp status */
diff --git a/drivers/net/ethernet/samsung/xgmac_dma.c 
b/drivers/net/ethernet/samsung/xgmac_dma.c
index 47945a3..384866c 100644
--- a/drivers/net/ethernet/samsung/xgmac_dma.c
+++ b/drivers/net/ethernet/samsung/xgmac_dma.c
@@ -85,14 +85,14 @@ static void xgmac_dma_channel_init(void __iomem *ioaddr, 
int cha_num,
 
/* program desc registers */
writel((dma_tx  32),
-   ioaddr + XGMAC_DMA_CHA_TXDESC_HADD_REG(cha_num));
+  ioaddr + XGMAC_DMA_CHA_TXDESC_HADD_REG(cha_num));
writel((dma_tx  0x),
-   ioaddr + XGMAC_DMA_CHA_TXDESC_LADD_REG(cha_num));
+  ioaddr + XGMAC_DMA_CHA_TXDESC_LADD_REG(cha_num));
 
writel((dma_rx  32),
-   ioaddr + XGMAC_DMA_CHA_RXDESC_HADD_REG(cha_num));
+  ioaddr + XGMAC_DMA_CHA_RXDESC_HADD_REG(cha_num));
writel((dma_rx  0x),
-   ioaddr + XGMAC_DMA_CHA_RXDESC_LADD_REG(cha_num));
+  ioaddr + XGMAC_DMA_CHA_RXDESC_LADD_REG(cha_num));
 
/* program

[PATCH 2/5] samsung: xgmac: Fix pr_level uses

2014-03-05 Thread Joe Perches 
Use pr_fmt to prefix messages consistently with samsung_xgmac: .
Add missing newlines.
Use print_hex_dump_debug.
Alignment neatening of pr_level uses.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/net/ethernet/samsung/xgmac_core.c |  8 ++--
 drivers/net/ethernet/samsung/xgmac_desc.c | 11 +++--
 drivers/net/ethernet/samsung/xgmac_ethtool.c  |  5 +-
 drivers/net/ethernet/samsung/xgmac_main.c | 69 +--
 drivers/net/ethernet/samsung/xgmac_mdio.c |  5 +-
 drivers/net/ethernet/samsung/xgmac_mtl.c  |  7 ++-
 drivers/net/ethernet/samsung/xgmac_platform.c | 15 +++---
 7 files changed, 67 insertions(+), 53 deletions(-)

diff --git a/drivers/net/ethernet/samsung/xgmac_core.c 
b/drivers/net/ethernet/samsung/xgmac_core.c
index 8fa2241..c5afba4 100644
--- a/drivers/net/ethernet/samsung/xgmac_core.c
+++ b/drivers/net/ethernet/samsung/xgmac_core.c
@@ -11,6 +11,8 @@
  * published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
+
 #include linux/export.h
 #include linux/io.h
 #include linux/netdevice.h
@@ -92,15 +94,15 @@ static void xgmac_core_pmt(void __iomem *ioaddr, unsigned 
long mode)
unsigned int pmt = 0;
 
if (mode  WAKE_MAGIC) {
-   pr_debug(GMAC: WOL Magic frame\n);
+   pr_debug(WOL Magic frame\n);
pmt |= PMT_MGPKT_EN;
}
if (mode  WAKE_UCAST) {
-   pr_debug(GMAC: WOL on global unicast\n);
+   pr_debug(WOL on global unicast\n);
pmt |= PMT_GUCAST_EN;
}
if (mode  (WAKE_MCAST | WAKE_BCAST)) {
-   pr_debug(GMAC: WOL on any other packet\n);
+   pr_debug(WOL on any other packet\n);
pmt |= PMT_RWKPKT_EN;
}
 
diff --git a/drivers/net/ethernet/samsung/xgmac_desc.c 
b/drivers/net/ethernet/samsung/xgmac_desc.c
index ddf3e94..ef25efd 100644
--- a/drivers/net/ethernet/samsung/xgmac_desc.c
+++ b/drivers/net/ethernet/samsung/xgmac_desc.c
@@ -9,6 +9,9 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
+
 #include linux/bitops.h
 #include linux/export.h
 #include linux/io.h
@@ -287,7 +290,7 @@ static int xgmac_rx_wbstatus(struct xgmac_rx_norm_desc *p,
x-overflow_error++;
break;
default:
-   pr_err(\tInvalid Error type\n);
+   pr_err(Invalid Error type\n);
break;
}
} else {
@@ -332,7 +335,7 @@ static int xgmac_rx_wbstatus(struct xgmac_rx_norm_desc *p,
x-dvlan_ocvlan_icvlan_pkt++;
break;
default:
-   pr_err(\tInvalid L2 Packet type\n);
+   pr_err(Invalid L2 Packet type\n);
break;
}
}
@@ -367,7 +370,7 @@ static int xgmac_rx_wbstatus(struct xgmac_rx_norm_desc *p,
x-ip6_unknown_pkt++;
break;
default:
-   pr_err(\tInvalid L3/L4 Packet type\n);
+   pr_err(Invalid L3/L4 Packet type\n);
break;
}
 
@@ -446,7 +449,7 @@ static void xgmac_rx_ctxt_wbstatus(struct 
xgmac_rx_ctxt_desc *p,
 static int xgmac_get_rx_ctxt_tstamp_status(struct xgmac_rx_ctxt_desc *p)
 {
if ((p-tstamp_hi == 0x)  (p-tstamp_lo == 0x)) {
-   pr_err(\tTime stamp corrupted\n);
+   pr_err(Time stamp corrupted\n);
return 0;
}
 
diff --git a/drivers/net/ethernet/samsung/xgmac_ethtool.c 
b/drivers/net/ethernet/samsung/xgmac_ethtool.c
index 3f0698a..576b23e 100644
--- a/drivers/net/ethernet/samsung/xgmac_ethtool.c
+++ b/drivers/net/ethernet/samsung/xgmac_ethtool.c
@@ -9,6 +9,9 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
+
 #include linux/clk.h
 #include linux/interrupt.h
 #include linux/kernel.h
@@ -200,7 +203,7 @@ static int xgmac_set_wol(struct net_device *dev, struct 
ethtool_wolinfo *wol)
return -EOPNOTSUPP;
 
if (wol-wolopts) {
-   pr_info(xgmac: wakeup enable\n);
+   pr_info(wakeup enable\n);
device_set_wakeup_enable(priv-device, true);
enable_irq_wake(priv-wol_irq);
} else {
diff --git a/drivers/net/ethernet/samsung/xgmac_main.c 
b/drivers/net/ethernet/samsung/xgmac_main.c
index 05853d2..a212abf 100644
--- a/drivers/net/ethernet/samsung/xgmac_main.c
+++ b/drivers/net/ethernet/samsung/xgmac_main.c
@@ -9,6 +9,9 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME :  fmt
+
 #include linux/clk.h
 #include linux/crc32.h

[PATCH 0/5] samsung: xgmac:

2014-03-05 Thread Joe Perches 
Mostly neatening and logging cleanups on top of the initial submittal.

Joe Perches (5):
  samsung: xgmac: Neatening
  samsung: xgmac: Fix pr_level uses
  samsung: xgmac: Use more current logging style
  samsung: xgmac: Neaten comments
  samsung: xgmac: Mostly whitespace neatening

 drivers/net/ethernet/samsung/xgmac_common.h   |  56 ++--
 drivers/net/ethernet/samsung/xgmac_core.c |  28 +-
 drivers/net/ethernet/samsung/xgmac_desc.c |  39 ++-
 drivers/net/ethernet/samsung/xgmac_desc.h |  16 +-
 drivers/net/ethernet/samsung/xgmac_dma.c  |  49 ++--
 drivers/net/ethernet/samsung/xgmac_dma.h  |  12 +-
 drivers/net/ethernet/samsung/xgmac_ethtool.c  |  95 ---
 drivers/net/ethernet/samsung/xgmac_main.c | 389 +-
 drivers/net/ethernet/samsung/xgmac_mdio.c |  58 ++--
 drivers/net/ethernet/samsung/xgmac_mtl.c  |  15 +-
 drivers/net/ethernet/samsung/xgmac_mtl.h  |  14 +-
 drivers/net/ethernet/samsung/xgmac_platform.c |  45 +--
 drivers/net/ethernet/samsung/xgmac_reg.h  | 190 ++---
 13 files changed, 502 insertions(+), 504 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] samsung: xgmac: Use more current logging style

2014-03-05 Thread Joe Perches 
Use netdev_level and netif_level where appropriate.
Remove else and unnecessary indentation around if goto else.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/net/ethernet/samsung/xgmac_ethtool.c  |   2 +-
 drivers/net/ethernet/samsung/xgmac_main.c | 185 --
 drivers/net/ethernet/samsung/xgmac_mdio.c |  13 +-
 drivers/net/ethernet/samsung/xgmac_platform.c |   4 +-
 4 files changed, 95 insertions(+), 109 deletions(-)

diff --git a/drivers/net/ethernet/samsung/xgmac_ethtool.c 
b/drivers/net/ethernet/samsung/xgmac_ethtool.c
index 576b23e..378f6f1 100644
--- a/drivers/net/ethernet/samsung/xgmac_ethtool.c
+++ b/drivers/net/ethernet/samsung/xgmac_ethtool.c
@@ -203,7 +203,7 @@ static int xgmac_set_wol(struct net_device *dev, struct 
ethtool_wolinfo *wol)
return -EOPNOTSUPP;
 
if (wol-wolopts) {
-   pr_info(wakeup enable\n);
+   netdev_info(dev, wakeup enable\n);
device_set_wakeup_enable(priv-device, true);
enable_irq_wake(priv-wol_irq);
} else {
diff --git a/drivers/net/ethernet/samsung/xgmac_main.c 
b/drivers/net/ethernet/samsung/xgmac_main.c
index a212abf..f642c99 100644
--- a/drivers/net/ethernet/samsung/xgmac_main.c
+++ b/drivers/net/ethernet/samsung/xgmac_main.c
@@ -267,9 +267,9 @@ static void xgmac_adjust_link(struct net_device *dev)
speed = XGMAC_SPEED_1G;
break;
default:
-   if (netif_msg_link(priv))
-   pr_err(%s: Speed (%d) not supported\n,
-  dev-name, phydev-speed);
+   netif_err(priv, link, dev,
+ Speed (%d) not supported\n,
+ phydev-speed);
}
 
priv-speed = phydev-speed;
@@ -323,12 +323,12 @@ static int xgmac_init_phy(struct net_device *ndev)
 
snprintf(phy_id_fmt, MII_BUS_ID_SIZE + 3, PHY_ID_FMT, bus_id,
 priv-plat-phy_addr);
-   pr_debug(%s: trying to attach to %s\n, __func__, phy_id_fmt);
+   netdev_dbg(ndev, %s: trying to attach to %s\n, __func__, phy_id_fmt);
 
phydev = phy_connect(ndev, phy_id_fmt, xgmac_adjust_link, phy_iface);
 
if (IS_ERR(phydev)) {
-   pr_err(%s: Could not attach to PHY\n, ndev-name);
+   netdev_err(ndev, Could not attach to PHY\n);
return PTR_ERR(phydev);
}
 
@@ -342,8 +342,8 @@ static int xgmac_init_phy(struct net_device *ndev)
return -ENODEV;
}
 
-   pr_debug(%s: %s: attached to PHY (UID 0x%x) Link = %d\n,
-__func__, ndev-name, phydev-phy_id, phydev-link);
+   netdev_dbg(ndev, %s: attached to PHY (UID 0x%x) Link = %d\n,
+  __func__, phydev-phy_id, phydev-link);
 
/* save phy device in private structure */
priv-phydev = phydev;
@@ -387,7 +387,7 @@ static int xgmac_init_rx_buffers(struct net_device *dev,
 
skb = __netdev_alloc_skb(dev, dma_buf_sz, GFP_KERNEL);
if (!skb) {
-   pr_err(%s: Rx init fails; skb is NULL\n, __func__);
+   netdev_err(dev, %s: Rx init fails; skb is NULL\n, __func__);
return -ENOMEM;
}
skb_reserve(skb, NET_IP_ALIGN);
@@ -397,9 +397,9 @@ static int xgmac_init_rx_buffers(struct net_device *dev,
dma_buf_sz, DMA_FROM_DEVICE);
 
if (dma_mapping_error(priv-device, rx_ring-rx_skbuff_dma[i])) {
-   pr_err(%s: DMA mapping error\n, __func__);
-   dev_kfree_skb_any(skb);
-   return -EINVAL;
+   netdev_err(dev, %s: DMA mapping error\n, __func__);
+   dev_kfree_skb_any(skb);
+   return -EINVAL;
}
 
p-rdes23.rx_rd_des23.buf2_addr = rx_ring-rx_skbuff_dma[i];
@@ -499,60 +499,51 @@ static int init_rx_ring(struct net_device *dev, u8 
queue_no,
/* Set the max buffer size according to the MTU. */
bfsize = ALIGN(dev-mtu + ETH_HLEN + ETH_FCS_LEN + NET_IP_ALIGN, 8);
 
-   if (netif_msg_probe(priv))
-   pr_debug(%s: bfsize %d\n, __func__, bfsize);
+   netif_dbg(priv, probe, dev, %s: bfsize %d\n, __func__, bfsize);
 
/* RX ring is not allcoated */
if (rx_ring == NULL) {
-   pr_err(No memory for RX queue\n);
+   netdev_err(dev, No memory for RX queue\n);
goto error;
-   } else {
-   /* assign queue number */
-   rx_ring-queue_no = queue_no;
-
-   /* allocate memory for RX descriptors */
-   rx_ring-dma_rx = dma_zalloc_coherent(priv-device,
-   rx_rsize * sizeof(struct xgmac_rx_norm_desc

[PATCH 5/5] samsung: xgmac: Mostly whitespace neatening

2014-03-05 Thread Joe Perches 
Alignment to parenthesis and adding parenthesis where appropriate.
Added a conversion of if (foo  x) bar ; else return baz to
if (foo = x) return baz; and unindented.

git diff -w shows trivial changes.
Added a few  80 column lines.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/net/ethernet/samsung/xgmac_common.h   |  32 ++---
 drivers/net/ethernet/samsung/xgmac_core.c |  19 ++-
 drivers/net/ethernet/samsung/xgmac_desc.c |  10 +-
 drivers/net/ethernet/samsung/xgmac_desc.h |  14 +-
 drivers/net/ethernet/samsung/xgmac_dma.c  |  14 +-
 drivers/net/ethernet/samsung/xgmac_dma.h  |  10 +-
 drivers/net/ethernet/samsung/xgmac_ethtool.c  |  23 ++--
 drivers/net/ethernet/samsung/xgmac_main.c |  95 +++---
 drivers/net/ethernet/samsung/xgmac_mdio.c |  33 +++--
 drivers/net/ethernet/samsung/xgmac_mtl.c  |   4 +-
 drivers/net/ethernet/samsung/xgmac_mtl.h  |  14 +-
 drivers/net/ethernet/samsung/xgmac_platform.c |  28 ++--
 drivers/net/ethernet/samsung/xgmac_reg.h  | 182 +-
 13 files changed, 240 insertions(+), 238 deletions(-)

diff --git a/drivers/net/ethernet/samsung/xgmac_common.h 
b/drivers/net/ethernet/samsung/xgmac_common.h
index 4c46504..47721b6 100644
--- a/drivers/net/ethernet/samsung/xgmac_common.h
+++ b/drivers/net/ethernet/samsung/xgmac_common.h
@@ -8,7 +8,7 @@
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
-*/
+ */
 
 #ifndef __XGMAC_COMMON_H__
 #define __XGMAC_COMMON_H__
@@ -167,21 +167,21 @@ enum dma_irq_status {
handle_rx = BIT(5),
 };
 
-#define NETIF_F_HW_VLAN_ALL (NETIF_F_HW_VLAN_CTAG_RX |\
-   NETIF_F_HW_VLAN_STAG_RX |\
-   NETIF_F_HW_VLAN_CTAG_TX |\
-   NETIF_F_HW_VLAN_STAG_TX |\
-   NETIF_F_HW_VLAN_CTAG_FILTER |\
-   NETIF_F_HW_VLAN_STAG_FILTER)
+#define NETIF_F_HW_VLAN_ALL (NETIF_F_HW_VLAN_CTAG_RX | \
+NETIF_F_HW_VLAN_STAG_RX |  \
+NETIF_F_HW_VLAN_CTAG_TX |  \
+NETIF_F_HW_VLAN_STAG_TX |  \
+NETIF_F_HW_VLAN_CTAG_FILTER |  \
+NETIF_F_HW_VLAN_STAG_FILTER)
 
 /* MMC control defines */
 #define XGMAC_MMC_CTRL_CNT_FRZ  0x0008
 
 /* XGMAC HW ADDR regs */
 #define XGMAC_ADDR_HIGH(reg)(((reg  15) ? 0x0800 : 0x0040) + \
-   (reg * 8))
+(reg * 8))
 #define XGMAC_ADDR_LOW(reg) (((reg  15) ? 0x0804 : 0x0044) + \
-   (reg * 8))
+(reg * 8))
 #define XGMAC_MAX_PERFECT_ADDRESSES 32 /* Maximum unicast perfect filtering */
 #define XGMAC_FRAME_FILTER   0x0004  /* Frame Filter */
 
@@ -207,7 +207,7 @@ enum dma_irq_status {
 #define MIN_MTU 68
 #define MAX_MTU 9000
 
-#define XGMAC_FOR_EACH_QUEUE(max_queues, queue_num) \
+#define XGMAC_FOR_EACH_QUEUE(max_queues, queue_num)\
for (queue_num = 0; queue_num  max_queues; queue_num++)
 
 #define DRV_VERSION 1.0.0
@@ -331,7 +331,7 @@ struct xgmac_hwtimestamp {
int (*init_systime)(void __iomem *ioaddr, u32 sec, u32 nsec);
int (*config_addend)(void __iomem *ioaddr, u32 addend);
int (*adjust_systime)(void __iomem *ioaddr, u32 sec, u32 nsec,
-  int add_sub);
+ int add_sub);
u64 (*get_systime)(void __iomem *ioaddr);
 };
 
@@ -353,14 +353,14 @@ struct xgmac_core_ops {
void (*dump_regs)(void __iomem *ioaddr);
/* Handle extra events on specific interrupts hw dependent */
int (*host_irq_status)(void __iomem *ioaddr,
-   struct xgmac_extra_stats *x);
+  struct xgmac_extra_stats *x);
/* Set power management mode (e.g. magic frame) */
void (*pmt)(void __iomem *ioaddr, unsigned long mode);
/* Set/Get Unicast MAC addresses */
void (*set_umac_addr)(void __iomem *ioaddr, unsigned char *addr,
-  unsigned int reg_n);
+ unsigned int reg_n);
void (*get_umac_addr)(void __iomem *ioaddr, unsigned char *addr,
-  unsigned int reg_n);
+ unsigned int reg_n);
void (*enable_rx)(void __iomem *ioaddr, bool enable);
void (*enable_tx)(void __iomem *ioaddr, bool enable);
 
@@ -369,7 +369,7 @@ struct xgmac_core_ops {
 
/* If supported then get the optional core features */
unsigned int (*get_hw_feature)(void __iomem *ioaddr,
-   unsigned char feature_index

[PATCH 4/5] samsung: xgmac: Neaten comments

2014-03-05 Thread Joe Perches 
Fix a couple of typos
Fix some comments that seem to have been kernel-doc style.
Fix alignment where noticed.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/net/ethernet/samsung/xgmac_core.c |  1 -
 drivers/net/ethernet/samsung/xgmac_desc.c | 14 +++---
 drivers/net/ethernet/samsung/xgmac_desc.h |  2 +-
 drivers/net/ethernet/samsung/xgmac_dma.c  |  3 ++-
 drivers/net/ethernet/samsung/xgmac_dma.h  |  2 +-
 drivers/net/ethernet/samsung/xgmac_main.c | 14 --
 drivers/net/ethernet/samsung/xgmac_reg.h  |  8 
 7 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/samsung/xgmac_core.c 
b/drivers/net/ethernet/samsung/xgmac_core.c
index c5afba4..40b1946 100644
--- a/drivers/net/ethernet/samsung/xgmac_core.c
+++ b/drivers/net/ethernet/samsung/xgmac_core.c
@@ -1,4 +1,3 @@
-
 /* 10G controller driver for Samsung SoCs
  *
  * Copyright (C) 2013 Samsung Electronics Co., Ltd.
diff --git a/drivers/net/ethernet/samsung/xgmac_desc.c 
b/drivers/net/ethernet/samsung/xgmac_desc.c
index ef25efd..2a7fd21 100644
--- a/drivers/net/ethernet/samsung/xgmac_desc.c
+++ b/drivers/net/ethernet/samsung/xgmac_desc.c
@@ -84,7 +84,7 @@ static void xgmac_release_tx_desc(struct xgmac_tx_norm_desc 
*p)
 
 /* Clear interrupt on tx frame completion. When this bit is
  * set an interrupt happens as soon as the frame is transmitted
-*/
+ */
 static void xgmac_clear_tx_ic(struct xgmac_tx_norm_desc *p)
 {
p-tdes23.tx_rd_des23.int_on_com = 0;
@@ -158,8 +158,8 @@ static void xgmac_tx_ctxt_desc_reset_ostc(struct 
xgmac_tx_ctxt_desc *p)
 
 /* Set IVLAN information */
 static void xgmac_tx_ctxt_desc_set_ivlantag(struct xgmac_tx_ctxt_desc *p,
-int is_ivlanvalid, int ivlan_tag,
-int ivlan_ctl)
+   int is_ivlanvalid, int ivlan_tag,
+   int ivlan_ctl)
 {
if (is_ivlanvalid) {
p-ivlan_tag_valid = is_ivlanvalid;
@@ -176,7 +176,7 @@ static int xgmac_tx_ctxt_desc_get_ivlantag(struct 
xgmac_tx_ctxt_desc *p)
 
 /* Set VLAN Tag */
 static void xgmac_tx_ctxt_desc_set_vlantag(struct xgmac_tx_ctxt_desc *p,
-   int is_vlanvalid, int vlan_tag)
+  int is_vlanvalid, int vlan_tag)
 {
if (is_vlanvalid) {
p-vltag_valid = is_vlanvalid;
@@ -214,7 +214,7 @@ static int xgmac_tx_ctxt_desc_get_cde(struct 
xgmac_tx_ctxt_desc *p)
 
 /* DMA RX descriptor ring initialization */
 static void xgmac_init_rx_desc(struct xgmac_rx_norm_desc *p, int disable_rx_ic,
-   int mode, int end)
+  int mode, int end)
 {
p-rdes23.rx_rd_des23.own_bit = 1;
if (disable_rx_ic)
@@ -413,7 +413,7 @@ static void xgmac_set_ctxt_rx_owner(struct 
xgmac_rx_ctxt_desc *p)
 
 /* Return the reception status looking at Context control information */
 static void xgmac_rx_ctxt_wbstatus(struct xgmac_rx_ctxt_desc *p,
-   struct xgmac_extra_stats *x)
+  struct xgmac_extra_stats *x)
 {
if (p-tstamp_dropped)
x-timestamp_dropped++;
@@ -445,7 +445,7 @@ static void xgmac_rx_ctxt_wbstatus(struct 
xgmac_rx_ctxt_desc *p,
x-rx_ptp_resv_msg_type++;
 }
 
-/* get rx timestamp status */
+/* Get rx timestamp status */
 static int xgmac_get_rx_ctxt_tstamp_status(struct xgmac_rx_ctxt_desc *p)
 {
if ((p-tstamp_hi == 0x)  (p-tstamp_lo == 0x)) {
diff --git a/drivers/net/ethernet/samsung/xgmac_desc.h 
b/drivers/net/ethernet/samsung/xgmac_desc.h
index 028b505..8433030 100644
--- a/drivers/net/ethernet/samsung/xgmac_desc.h
+++ b/drivers/net/ethernet/samsung/xgmac_desc.h
@@ -14,7 +14,7 @@
 
 #define XGMAC_DESC_SIZE_BYTES  16
 
-/* forward declatarion */
+/* forward declaration */
 struct xgmac_extra_stats;
 
 /* Transmit checksum insertion control */
diff --git a/drivers/net/ethernet/samsung/xgmac_dma.c 
b/drivers/net/ethernet/samsung/xgmac_dma.c
index 384866c..9a22990a 100644
--- a/drivers/net/ethernet/samsung/xgmac_dma.c
+++ b/drivers/net/ethernet/samsung/xgmac_dma.c
@@ -20,9 +20,10 @@
 #include xgmac_dma.h
 #include xgmac_reg.h
 #include xgmac_desc.h
+
 /* DMA core initialization */
 static int xgmac_dma_init(void __iomem *ioaddr, int fix_burst,
-int burst_map, int adv_addr_mode)
+ int burst_map, int adv_addr_mode)
 {
int retry_count = 10;
u32 reg_val;
diff --git a/drivers/net/ethernet/samsung/xgmac_dma.h 
b/drivers/net/ethernet/samsung/xgmac_dma.h
index 2e76ae0..022fd2b 100644
--- a/drivers/net/ethernet/samsung/xgmac_dma.h
+++ b/drivers/net/ethernet/samsung/xgmac_dma.h
@@ -12,7 +12,7 @@
 #ifndef __XGMAC_DMA_H__
 #define __XGMAC_DMA_H__
 
-/* forward declatarion */
+/* forward declaration */
 struct xgmac_extra_stats;
 
 #define

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 20:26 +0200, Uwe Kleine-König wrote:
 Hello Joe,
 
 On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
  Emitting an OOM message isn't necessary after input_allocate_device
  as there's a generic OOM and a dump_stack already done.
  
  [...]
  Signed-off-by: Joe Perches j...@perches.com
  diff --git a/drivers/input/joystick/as5011.c 
  b/drivers/input/joystick/as5011.c
  index 005d852..3b9c709 100644
  --- a/drivers/input/joystick/as5011.c
  +++ b/drivers/input/joystick/as5011.c
  @@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
  as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
  input_dev = input_allocate_device();
  if (!as5011 || !input_dev) {
  -   dev_err(client-dev,
  -   Can't allocate memory for device structure\n);
 Don't know if that can happen, but if as5011 is NULL but input_dev isn't
 the message would still be sensible, wouldn't it? There are several more
 that suffer the same problem.

Any k.alloc without __GFP_NOWARN does a generic OOM message
and a dump_stack() so there could already be 2 messages anyway.

cheers, Joe

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
 Hi Joe,
 
 On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
  Emitting an OOM message isn't necessary after input_allocate_device
  as there's a generic OOM and a dump_stack already done.
 
 No, please don't. The kzalloc may get changed in the future to not dump
 stack (that was added originally because not everyone was handling OOM
 properly, right?), input core might get changed to use something else
 than kzalloc, etc, etc.
 
 The majority of errors use dev_err so we also get idea what device
 failed (if there are several), and more.

I think that's not valuable as input_allocate_device already has
dozens of locations that don't emit a specific OOM and centralizing
the location for any generic message would work anyway.



--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 20:46 +0200, Uwe Kleine-König wrote:
 On Thu, Oct 24, 2013 at 11:43:38AM -0700, Joe Perches wrote:
[]
  Any k.alloc without __GFP_NOWARN does a generic OOM message
  and a dump_stack() so there could already be 2 messages anyway.
 Then mention that in the commit log if you still want this patch?!

How do you think this commit message should be changed:

Emitting an OOM message isn't necessary after input_allocate_device
as there's a generic OOM and a dump_stack already done.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-24 Thread Joe Perches 
On Thu, 2013-10-24 at 12:10 -0700, Dmitry Torokhov wrote:
 On Thu, Oct 24, 2013 at 11:45:39AM -0700, Joe Perches wrote:
  On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
   Hi Joe,
   
   On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
Emitting an OOM message isn't necessary after input_allocate_device
as there's a generic OOM and a dump_stack already done.
   
   No, please don't. The kzalloc may get changed in the future to not dump
   stack (that was added originally because not everyone was handling OOM
   properly, right?), input core might get changed to use something else
   than kzalloc, etc, etc.
   
   The majority of errors use dev_err so we also get idea what device
   failed (if there are several), and more.
  
  I think that's not valuable as input_allocate_device already has
  dozens of locations that don't emit a specific OOM and centralizing
  the location for any generic message would work anyway.
 
 Not having diagnostic messages in some of the drivers is hardly a
 justification to remove them from everywhere else.

But standardization is.

If a diagnostic message is really desired
without the already existing dump_stack(),
it's a small matter of adding something like:

 drivers/input/input.c | 26 +++---
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index c044699..98570c2 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1736,19 +1736,23 @@ struct input_dev *input_allocate_device(void)
 {
struct input_dev *dev;
 
-   dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL);
-   if (dev) {
-   dev-dev.type = input_dev_type;
-   dev-dev.class = input_class;
-   device_initialize(dev-dev);
-   mutex_init(dev-mutex);
-   spin_lock_init(dev-event_lock);
-   INIT_LIST_HEAD(dev-h_list);
-   INIT_LIST_HEAD(dev-node);
-
-   __module_get(THIS_MODULE);
+   dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL | __GFP_NOWARN);
+   if (!dev) {
+   pr_err(%pf: input_allocate_device failed\n,
+  __builtin_return_address(1));
+   return NULL;
}
 
+   dev-dev.type = input_dev_type;
+   dev-dev.class = input_class;
+   device_initialize(dev-dev);
+   mutex_init(dev-mutex);
+   spin_lock_init(dev-event_lock);
+   INIT_LIST_HEAD(dev-h_list);
+   INIT_LIST_HEAD(dev-node);
+
+   __module_get(THIS_MODULE);
+
return dev;
 }
 EXPORT_SYMBOL(input_allocate_device);



--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] input: Remove OOM message after input_allocate_device

2013-10-23 Thread Joe Perches 
Emitting an OOM message isn't necessary after input_allocate_device
as there's a generic OOM and a dump_stack already done.

Signed-off-by: Joe Perches j...@perches.com
---
 drivers/input/joystick/as5011.c | 2 --
 drivers/input/joystick/db9.c| 1 -
 drivers/input/joystick/gamecon.c| 4 +---
 drivers/input/joystick/turbografx.c | 1 -
 drivers/input/joystick/walkera0701.c| 1 -
 drivers/input/keyboard/amikbd.c | 4 +---
 drivers/input/keyboard/davinci_keyscan.c| 1 -
 drivers/input/keyboard/gpio_keys.c  | 1 -
 drivers/input/keyboard/lpc32xx-keys.c   | 1 -
 drivers/input/keyboard/max7359_keypad.c | 1 -
 drivers/input/keyboard/mcs_touchkey.c   | 1 -
 drivers/input/keyboard/mpr121_touchkey.c| 1 -
 drivers/input/keyboard/nomadik-ske-keypad.c | 1 -
 drivers/input/keyboard/opencores-kbd.c  | 1 -
 drivers/input/keyboard/pmic8xxx-keypad.c| 1 -
 drivers/input/keyboard/pxa27x_keypad.c  | 1 -
 drivers/input/keyboard/pxa930_rotary.c  | 1 -
 drivers/input/keyboard/qt1070.c | 1 -
 drivers/input/keyboard/qt2160.c | 1 -
 drivers/input/keyboard/sh_keysc.c   | 1 -
 drivers/input/keyboard/tc3589x-keypad.c | 1 -
 drivers/input/keyboard/tnetv107x-keypad.c   | 1 -
 drivers/input/keyboard/w90p910_keypad.c | 1 -
 drivers/input/misc/88pm80x_onkey.c  | 1 -
 drivers/input/misc/88pm860x_onkey.c | 1 -
 drivers/input/misc/arizona-haptics.c| 4 +---
 drivers/input/misc/atlas_btns.c | 4 +---
 drivers/input/misc/da9052_onkey.c   | 1 -
 drivers/input/misc/da9055_onkey.c   | 4 +---
 drivers/input/misc/ideapad_slidebar.c   | 1 -
 drivers/input/misc/ims-pcu.c| 7 +--
 drivers/input/misc/kxtj9.c  | 4 +---
 drivers/input/misc/max8997_haptic.c | 1 -
 drivers/input/misc/mc13783-pwrbutton.c  | 4 +---
 drivers/input/misc/mpu3050.c| 1 -
 drivers/input/misc/pcf8574_keypad.c | 1 -
 drivers/input/misc/pm8xxx-vibrator.c| 1 -
 drivers/input/misc/pmic8xxx-pwrkey.c| 1 -
 drivers/input/misc/pwm-beeper.c | 1 -
 drivers/input/misc/twl4030-pwrbutton.c  | 4 +---
 drivers/input/misc/twl6040-vibra.c  | 1 -
 drivers/input/mouse/appletouch.c| 4 +---
 drivers/input/mouse/bcm5974.c   | 4 +---
 drivers/input/mouse/cyapa.c | 4 +---
 drivers/input/mouse/inport.c| 1 -
 drivers/input/mouse/logibm.c| 1 -
 drivers/input/mouse/pc110pad.c  | 1 -
 drivers/input/mouse/pxa930_trkball.c| 1 -
 drivers/input/tablet/aiptek.c   | 5 +
 drivers/input/tablet/gtco.c | 1 -
 drivers/input/touchscreen/88pm860x-ts.c | 1 -
 drivers/input/touchscreen/atmel_mxt_ts.c| 1 -
 drivers/input/touchscreen/atmel_tsadcc.c| 1 -
 drivers/input/touchscreen/bu21013_ts.c  | 1 -
 drivers/input/touchscreen/cyttsp4_core.c| 2 --
 drivers/input/touchscreen/da9034-ts.c   | 1 -
 drivers/input/touchscreen/edt-ft5x06.c  | 1 -
 drivers/input/touchscreen/eeti_ts.c | 5 +
 drivers/input/touchscreen/htcpen.c  | 1 -
 drivers/input/touchscreen/intel-mid-touch.c | 1 -
 drivers/input/touchscreen/lpc32xx_ts.c  | 1 -
 drivers/input/touchscreen/mcs5000_ts.c  | 1 -
 drivers/input/touchscreen/migor_ts.c| 1 -
 drivers/input/touchscreen/mk712.c   | 1 -
 drivers/input/touchscreen/pixcir_i2c_ts.c   | 1 -
 drivers/input/touchscreen/s3c2410_ts.c  | 1 -
 drivers/input/touchscreen/ti_am335x_tsc.c   | 1 -
 drivers/input/touchscreen/tnetv107x-ts.c| 1 -
 68 files changed, 14 insertions(+), 103 deletions(-)

diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
index 005d852..3b9c709 100644
--- a/drivers/input/joystick/as5011.c
+++ b/drivers/input/joystick/as5011.c
@@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
input_dev = input_allocate_device();
if (!as5011 || !input_dev) {
-   dev_err(client-dev,
-   Can't allocate memory for device structure\n);
error = -ENOMEM;
goto err_free_mem;
}
diff --git a/drivers/input/joystick/db9.c b/drivers/input/joystick/db9.c
index 8e7de5c..b0f18f2 100644
--- a/drivers/input/joystick/db9.c
+++ b/drivers/input/joystick/db9.c
@@ -609,7 +609,6 @@ static struct db9 __init *db9_probe(int parport, int mode)
 
db9-dev[i] = input_dev = input_allocate_device();
if (!input_dev) {
-   printk(KERN_ERR db9.c: Not enough memory for input 
device\n);
err = -ENOMEM;
goto err_unreg_devs;
}
diff --git a/drivers/input/joystick/gamecon.c b/drivers/input/joystick/gamecon.c
index e68e497..ded4f23 100644

[PATCH 0/8] treewide: Remove OOM message after input_alloc_device

2013-10-23 Thread Joe Perches 
Joe Perches (8):
  Documentation: Remove OOM message after input_allocate_device
  cell: Remove OOM message after input_allocate_device
  hid: Remove OOM message after input_allocate_device
  input: Remove OOM message after input_allocate_device
  media: Remove OOM message after input_allocate_device
  platform:x86: Remove OOM message after input_allocate_device
  staging: Remove OOM message after input_allocate_device
  sound: Remove OOM message after input_allocate_device

 Documentation/input/input-programming.txt | 1 -
 arch/powerpc/platforms/cell/cbe_powerbutton.c | 1 -
 drivers/hid/hid-input.c   | 1 -
 drivers/hid/hid-picolcd_core.c| 5 ++---
 drivers/input/joystick/as5011.c   | 2 --
 drivers/input/joystick/db9.c  | 1 -
 drivers/input/joystick/gamecon.c  | 4 +---
 drivers/input/joystick/turbografx.c   | 1 -
 drivers/input/joystick/walkera0701.c  | 1 -
 drivers/input/keyboard/amikbd.c   | 4 +---
 drivers/input/keyboard/davinci_keyscan.c  | 1 -
 drivers/input/keyboard/gpio_keys.c| 1 -
 drivers/input/keyboard/lpc32xx-keys.c | 1 -
 drivers/input/keyboard/max7359_keypad.c   | 1 -
 drivers/input/keyboard/mcs_touchkey.c | 1 -
 drivers/input/keyboard/mpr121_touchkey.c  | 1 -
 drivers/input/keyboard/nomadik-ske-keypad.c   | 1 -
 drivers/input/keyboard/opencores-kbd.c| 1 -
 drivers/input/keyboard/pmic8xxx-keypad.c  | 1 -
 drivers/input/keyboard/pxa27x_keypad.c| 1 -
 drivers/input/keyboard/pxa930_rotary.c| 1 -
 drivers/input/keyboard/qt1070.c   | 1 -
 drivers/input/keyboard/qt2160.c   | 1 -
 drivers/input/keyboard/sh_keysc.c | 1 -
 drivers/input/keyboard/tc3589x-keypad.c   | 1 -
 drivers/input/keyboard/tnetv107x-keypad.c | 1 -
 drivers/input/keyboard/w90p910_keypad.c   | 1 -
 drivers/input/misc/88pm80x_onkey.c| 1 -
 drivers/input/misc/88pm860x_onkey.c   | 1 -
 drivers/input/misc/arizona-haptics.c  | 4 +---
 drivers/input/misc/atlas_btns.c   | 4 +---
 drivers/input/misc/da9052_onkey.c | 1 -
 drivers/input/misc/da9055_onkey.c | 4 +---
 drivers/input/misc/ideapad_slidebar.c | 1 -
 drivers/input/misc/ims-pcu.c  | 7 +--
 drivers/input/misc/kxtj9.c| 4 +---
 drivers/input/misc/max8997_haptic.c   | 1 -
 drivers/input/misc/mc13783-pwrbutton.c| 4 +---
 drivers/input/misc/mpu3050.c  | 1 -
 drivers/input/misc/pcf8574_keypad.c   | 1 -
 drivers/input/misc/pm8xxx-vibrator.c  | 1 -
 drivers/input/misc/pmic8xxx-pwrkey.c  | 1 -
 drivers/input/misc/pwm-beeper.c   | 1 -
 drivers/input/misc/twl4030-pwrbutton.c| 4 +---
 drivers/input/misc/twl6040-vibra.c| 1 -
 drivers/input/mouse/appletouch.c  | 4 +---
 drivers/input/mouse/bcm5974.c | 4 +---
 drivers/input/mouse/cyapa.c   | 4 +---
 drivers/input/mouse/inport.c  | 1 -
 drivers/input/mouse/logibm.c  | 1 -
 drivers/input/mouse/pc110pad.c| 1 -
 drivers/input/mouse/pxa930_trkball.c  | 1 -
 drivers/input/tablet/aiptek.c | 5 +
 drivers/input/tablet/gtco.c   | 1 -
 drivers/input/touchscreen/88pm860x-ts.c   | 1 -
 drivers/input/touchscreen/atmel_mxt_ts.c  | 1 -
 drivers/input/touchscreen/atmel_tsadcc.c  | 1 -
 drivers/input/touchscreen/bu21013_ts.c| 1 -
 drivers/input/touchscreen/cyttsp4_core.c  | 2 --
 drivers/input/touchscreen/da9034-ts.c | 1 -
 drivers/input/touchscreen/edt-ft5x06.c| 1 -
 drivers/input/touchscreen/eeti_ts.c   | 5 +
 drivers/input/touchscreen/htcpen.c| 1 -
 drivers/input/touchscreen/intel-mid-touch.c   | 1 -
 drivers/input/touchscreen/lpc32xx_ts.c| 1 -
 drivers/input/touchscreen/mcs5000_ts.c| 1 -
 drivers/input/touchscreen/migor_ts.c  | 1 -
 drivers/input/touchscreen/mk712.c | 1 -
 drivers/input/touchscreen/pixcir_i2c_ts.c | 1 -
 drivers/input/touchscreen/s3c2410_ts.c| 1 -
 drivers/input/touchscreen/ti_am335x_tsc.c | 1 -
 drivers/input/touchscreen/tnetv107x-ts.c  | 1 -
 drivers/media/rc/imon.c   | 8 ++--
 drivers/media/usb/em28xx/em28xx-input.c   | 4 +---
 drivers/media/usb/pwc/pwc-if.c| 1 -
 drivers/platform/x86/asus-laptop.c| 5 ++---
 drivers/platform/x86/eeepc-laptop.c   | 4 +---
 drivers/platform/x86/ideapad-laptop.c | 4 +---
 drivers/platform/x86/intel_mid_powerbtn.c | 4 +---
 drivers/platform/x86/panasonic-laptop.c   | 5 +
 drivers/platform/x86/thinkpad_acpi.c  | 1 -
 drivers/platform/x86/topstar-laptop.c | 4 +---
 drivers/platform/x86/toshiba_acpi.c   | 4 +---
 drivers/staging/cptm1217

Re: [PATCH 1/3] clk: exynos4: Make exynos4_plls static

2013-08-06 Thread Joe Perches 
On Wed, 2013-08-07 at 08:51 +0530, Sachin Kamat wrote:
 +CC Joe Perches
 
 On 7 August 2013 01:32, Russell King - ARM Linux li...@arm.linux.org.uk 
 wrote:
  Also note:
 
  On Tue, Aug 06, 2013 at 05:01:13PM +0530, Sachin Kamat wrote:
  @@ -984,7 +984,7 @@ static __initdata struct of_device_id ext_clk_match[] 
  = {
 
  For the declaration above...
 
  -struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
  +static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
 
  And this one... __initdata should come just before the '=', not at the
  start, not in the middle and not before the variable.
 
  The reasoning is that with how you have it above, the attributes are
  applied to the structure.  You want to apply the attributes to the
  declaration instead, so it should come after the variable name.
 
  So, for example:
 
  struct foo *foo __attribute__((section(.foo))) = (void *)1;
 
  will place the foo variable into a section called .foo, but:
 
  struct __attribute__((section(.foo))) foo *foo = (void *)1;
 
  will place foo into the normal .data section.
 
  So, the rule with variable declarations is that the __ specifiers we
  have as macros in the kernel always come after the variable name being
  declared and nowhere else.  We consider anywhere else buggy.
 
 Thanks for this useful tip, Russell. There are several instances in
 the kernel where these attributes are used at the beginning of the
 variable declaration.
 
 Probably it would be useful to add this to checkpatch. Joe?

I think Russell is using the royal We.

http://en.wikipedia.org/wiki/Majestic_plural

There's no way for checkpatch to look at an __foo use
and determine it should be before or after a variable.

__scanf, __printf, __cold, etc are often place before
declarations.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] spi: s3c64xx: fix checkpatch error and warnings

2013-07-15 Thread Joe Perches 
On Mon, 2013-07-15 at 15:11 +0900, Jingoo Han wrote:
[]
 diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
[]
 @@ -338,8 +338,10 @@ static int acquire_dma(struct s3c64xx_spi_driver_data 
 *sdd)
[]
 - sdd-rx_dma.ch = (void *)sdd-ops-request(sdd-rx_dma.dmach, req, 
 dev, rx);
 - sdd-tx_dma.ch = (void *)sdd-ops-request(sdd-tx_dma.dmach, req, 
 dev, tx);
 + sdd-rx_dma.ch = (void *)sdd-ops-request(sdd-rx_dma.dmach,
 + req, dev, rx);
 + sdd-tx_dma.ch = (void *)sdd-ops-request(sdd-tx_dma.dmach,
 + req, dev, tx);

There should be sparse errors here.
sdd-ops-request is unsigned int, not unsigned long.
Care to fix the cast of unsigned to pointer too?

 @@ -439,7 +441,7 @@ static int s3c64xx_spi_prepare_transfer(struct spi_master 
 *spi)
  
   /* Acquire DMA channels */
   sdd-rx_dma.ch = dma_request_slave_channel_compat(mask, filter,
 - (void*)sdd-rx_dma.dmach, dev, rx);
 + (void *)sdd-rx_dma.dmach, dev, rx);

It seems unsigned to pointer conversions are pretty
rampant in this code.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/2] spi: s3c64xx: fix checkpatch error and warnings

2013-07-15 Thread Joe Perches 
On Tue, 2013-07-16 at 08:23 +0900, Jingoo Han wrote:
 On Monday, July 15, 2013 9:10 PM, Joe Perches wrote:
  On Mon, 2013-07-15 at 15:11 +0900, Jingoo Han wrote:
  []
   diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
  []
   @@ -338,8 +338,10 @@ static int acquire_dma(struct 
   s3c64xx_spi_driver_data *sdd)
  []
   - sdd-rx_dma.ch = (void *)sdd-ops-request(sdd-rx_dma.dmach, req, 
   dev, rx);
   - sdd-tx_dma.ch = (void *)sdd-ops-request(sdd-tx_dma.dmach, req, 
   dev, tx);
   + sdd-rx_dma.ch = (void *)sdd-ops-request(sdd-rx_dma.dmach,
   + req, dev, rx);
   + sdd-tx_dma.ch = (void *)sdd-ops-request(sdd-tx_dma.dmach,
   + req, dev, tx);
  
  There should be sparse errors here.
  sdd-ops-request is unsigned int, not unsigned long.
  Care to fix the cast of unsigned to pointer too?
 
 OK, I will fix it as below:
 
 sdd-rx_dma.ch = (unsigned long *)sdd-ops-request(sdd-rx_dma.dmach,
   req, dev, rx);
 sdd-tx_dma.ch = (unsigned long *)sdd-ops-request(sdd-tx_dma.dmach,
   req, dev, tx);

Which should give you a different error.

The canonical (Small C, not the Ubuntu company)
way to do this is to first cast to unsigned long
then cast to pointer type.

sdd-tx_dma.ch = (unsigned long *)(unsigned long)sdd-ops-request(etc...)

  It seems unsigned to pointer conversions are pretty
  rampant in this code.
 
 It seems so.
 I will look into it, later.

No worries.  Whenever you get 'round to it.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] MAINTAINERS: Add Samsung pinctrl entries

2013-06-13 Thread Joe Perches 
On Thu, 2013-06-13 at 19:32 +0200, Tomasz Figa wrote:
[]
 Similarly for Kukjin, he's listed as the main Samsung maintainer already, 
 but AFAIK in this case the script can't infer this based on directory 
 structure. Let's see how it's done for other Samsung drivers:

A pattern could be added that matches any file with
exynos in it.

N: Files and directories with regex patterns.
   N:   [^a-z]tegra all files whose path contains the word tegra
   One pattern per line.  Multiple N: lines acceptable.

like:

N:  exynos
or
N:  drivers/pinctrl/.*exynos


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5 RESEND] thermal: exynos: Miscellaneous fixes to support falling threshold interrupt

2013-01-06 Thread Joe Perches 
On Sun, 2013-01-06 at 15:50 -0800, Amit Daniel Kachhap wrote:
 Below fixes are done to support falling threshold interrupt,
 * Falling interrupt status macro corrected according to exynos5 data sheet.
 * The get trend function modified to calculate trip temperature correctly.
 * The clearing of interrupt status in the isr is now done after handling
   the event that caused the interrupt.
[]
 diff --git a/drivers/thermal/exynos_thermal.c 
 b/drivers/thermal/exynos_thermal.c
[]
 @@ -373,12 +373,19 @@ static int exynos_get_temp(struct thermal_zone_device 
 *thermal,
  static int exynos_get_trend(struct thermal_zone_device *thermal,
   int trip, enum thermal_trend *trend)
  {
 - if (thermal-temperature = trip)
 + int ret = 0;

Unnecessary initialization

 + unsigned long trip_temp;
 +
 + ret = exynos_get_trip_temp(thermal, trip, trip_temp);
 + if (ret  0)
 + return ret;
 +
 + if (thermal-temperature = trip_temp)
   *trend = THERMAL_TREND_RAISING;
   else
   *trend = THERMAL_TREND_DROPPING;

THERMAL_TREND_STABLE ?

  
 - return 0;
 + return ret;

return 0 is clearer.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] serial: samsung: Add support serial for EXYNOS4212 and EXYNOS4412

2012-01-30 Thread Joe Perches 
On Tue, 2012-01-31 at 15:25 +0900, Kukjin Kim wrote:
 Kukjin Kim wrote:
  This should be added for EXYNOS4212 and EXYNOS4412 SoCs.
  
  Cc: Thomas Abraham thomas.abra...@linaro.org
  Cc: Greg Kroah-Hartman gre...@suse.de
  Cc: sta...@kerne.org
 
 Oops, should be 'Cc: sta...@kernel.org'

You mean it should be 'cc: sta...@vger.kernel.org'


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 resend2] s3c/s3c24xx: arm: leds: Make s3c24xx LEDS driver use gpiolib

2011-12-13 Thread Joe Perches 
On Tue, 2011-12-13 at 22:22 +0200, Denis Kuzmenko wrote:
 Make s3c24xx LEDS driver use gpiolib. Fix error disabling pull during probe.
[]
 diff --git a/drivers/leds/leds-s3c24xx.c b/drivers/leds/leds-s3c24xx.c
[]
 @@ -45,23 +45,35 @@ static void s3c24xx_led_set(struct led_classdev *led_cdev,
 + /*
 +  * ensure value is 0 or 1 to use it with bitwise XOR (^)
 +  * (only 100% brightness is supported)
 +  */
 + value = value ? 1 : 0;

Maybe value = !!value;

 + /* no point in having a pull-up as we are always driving */
 + s3c_gpio_setpull(pdata-gpio, S3C_GPIO_PULL_NONE);
 + ret = gpio_direction_output(pdata-gpio,
 + !!(pdata-flags  S3C24XX_LEDF_ACTLOW));

akin to this use.


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 resend2] s3c/s3c24xx: arm: leds: Make s3c24xx LEDS driver use gpiolib

2011-12-13 Thread Joe Perches 
On Tue, 2011-12-13 at 23:21 +0200, Denis Kuzmenko wrote:
 Hi Joe,
 Thank you for review.
 
 On 12/13/2011 11:06 PM, Joe Perches wrote:
  On Tue, 2011-12-13 at 22:22 +0200, Denis Kuzmenko wrote:
  Make s3c24xx LEDS driver use gpiolib. Fix error disabling pull during 
  probe.
  []
  diff --git a/drivers/leds/leds-s3c24xx.c b/drivers/leds/leds-s3c24xx.c
  []
  @@ -45,23 +45,35 @@ static void s3c24xx_led_set(struct led_classdev 
  *led_cdev,
  +  /*
  +   * ensure value is 0 or 1 to use it with bitwise XOR (^)
  +   * (only 100% brightness is supported)
  +   */
  +  value = value ? 1 : 0;
  
  Maybe value = !!value;
 
 IMHO my variant is better readable.
 
  +  /* no point in having a pull-up as we are always driving */
  +  s3c_gpio_setpull(pdata-gpio, S3C_GPIO_PULL_NONE);
  +  ret = gpio_direction_output(pdata-gpio,
  +  !!(pdata-flags  S3C24XX_LEDF_ACTLOW));
  
  akin to this use.
 
 There is no so much space for implementation used before so I've used
 shorter notation, again, for better readability.

Your choice, but I think consistency is better.

The LEDF_ACTLOW use is dependent on it being #defined
to 1 when using ^.  I think that's unintelligible.

cheers, Joe

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 resend2] s3c/s3c24xx: arm: leds: Make s3c24xx LEDS driver use gpiolib

2011-12-13 Thread Joe Perches 
On Tue, 2011-12-13 at 23:33 +0200, Denis Kuzmenko wrote:
 On 12/13/2011 11:28 PM, Joe Perches wrote:
  The LEDF_ACTLOW use is dependent on it being #defined
  to 1 when using ^.  I think that's unintelligible.
 Sorry, can't understand. Can you please clarify last two sentences?

s3c2410_gpio_setpin(pd-gpio, (value ? 1 : 0) ^
(pd-flags  S3C24XX_LEDF_ACTLOW));

I think this should be:
s3c2410_gpio_setpin(pd-gpio,
!!value ^ !!(pd-flags  S3C24XX_LEDF_ACTLOW));

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 resend2] s3c/s3c24xx: arm: leds: Make s3c24xx LEDS driver use gpiolib

2011-12-13 Thread Joe Perches 
On Wed, 2011-12-14 at 00:05 +0200, Denis Kuzmenko wrote:
 On 12/13/2011 11:40 PM, Joe Perches wrote:
  On Tue, 2011-12-13 at 23:33 +0200, Denis Kuzmenko wrote:
  On 12/13/2011 11:28 PM, Joe Perches wrote:
  The LEDF_ACTLOW use is dependent on it being #defined
  to 1 when using ^.  I think that's unintelligible.
  Sorry, can't understand. Can you please clarify last two sentences?
  s3c2410_gpio_setpin(pd-gpio, (value ? 1 : 0) ^
  (pd-flags  S3C24XX_LEDF_ACTLOW));
  I think this should be:
  s3c2410_gpio_setpin(pd-gpio,
  !!value ^ !!(pd-flags  S3C24XX_LEDF_ACTLOW));
 So you've found a BUG.

It's not really a bug, it's more lack of consistency
style defect.  It's also an example of why I prefer
consistent style to equivalent but different styles.

 As I understand my code will behave wrong if
 S3C24XX_LEDF_ACTLOW value will be changed.

 Thank you, I'll make a new version.

cheers, Joe

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] DMA: PL330: Fix build warning

2011-11-10 Thread Joe Perches 
On Thu, 2011-11-10 at 15:14 +0530, Vinod Koul wrote:
 On Thu, 2011-11-03 at 15:48 +0900, Boojin Kim wrote:
  This patch adds to fix the build warning as following.
  
  drivers/dma/pl330.c: In function 'pl330_probe':
  drivers/dma/pl330.c:859: warning: comparison of distinct pointer types 
  lacks a cast
  
  Signed-off-by: Boojin Kim boojin@samsung.com
  ---
   drivers/dma/pl330.c |3 ++-
   1 files changed, 2 insertions(+), 1 deletions(-)
  
  diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
  index 621134f..379d928 100644
  --- a/drivers/dma/pl330.c
  +++ b/drivers/dma/pl330.c
  @@ -856,7 +856,8 @@ pl330_probe(struct amba_device *adev, const struct 
  amba_id *id)
  INIT_LIST_HEAD(pd-channels);
   
  /* Initialize channel parameters */
  -   num_chan = max(pdat ? pdat-nr_valid_peri : 0, (u8)pi-pcfg.num_chan);
  +   num_chan = max(pdat ? (int)pdat-nr_valid_peri : 0,
  +   (int)pi-pcfg.num_chan);
  pdmac-peripherals = kzalloc(num_chan * sizeof(*pch), GFP_KERNEL);
   
  for (i = 0; i  num_chan; i++) {
 
 Applied Thanks

Perhaps
max_t(int, pdat ? pdat-nr_valid_peri : 0, pi-pcfg.num_chan);


--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html