Hi, > From: linux-acpi-owner@xxxxxxxxxxxxxxx [mailto:linux-acpi-owner@xxxxxxxxxxxxxxx] On Behalf Of Ashwin Chaugule > Sent: Friday, September 05, 2014 10:24 AM > > Hi Lv, > > On 4 September 2014 21:56, Zheng, Lv <lv.zheng@xxxxxxxxx> wrote: > >> + spin_lock_irqsave(&chan->lock, flags); > >> + chan->msg_free = 0; > >> + chan->msg_count = 0; > >> + chan->active_req = NULL; > >> + chan->cl = cl; > >> + init_completion(&chan->tx_complete); > >> + > >> + if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone) > > > > The tab should be a space here? > > > > Not sure where you're seeing the tab. It is a tab here after " txdone_method". > >> +static bool pcc_tx_done(struct mbox_chan *chan) > >> +{ > >> + struct acpi_pcct_subspace *pcct_ss = chan->con_priv; > >> + struct acpi_pcct_shared_memory *generic_comm_base = > >> + (struct acpi_pcct_shared_memory *) pcct_ss->base_address; > >> + u16 cmd_delay = pcct_ss->min_turnaround_time; > > > > Should these parameters be acquired with chan-lock held? > > Is that possible to change them during runtime? > > > > No. These parameters don't change. OK. Then that's up to you. :-) > > >> +/* Channel lock is already held by mbox controller code. */ > >> +static int pcc_send_data(struct mbox_chan *chan, void *data) > >> +{ > >> + struct acpi_pcct_subspace *pcct_ss = chan->con_priv; > >> + struct acpi_pcct_shared_memory *generic_comm_base = > >> + (struct acpi_pcct_shared_memory *) pcct_ss->base_address; > > > > > > Ditto. > > > > Same as above. > > >> +out_err: > >> + return ACPI_SUCCESS(status) ? 1 : 0; > > > > Should the return value of 1 here be -EINVAL? > > > > I dont think it really matters, the caller returns -EINVAL anyway. This is not a "bool" return value or "reversed bool" return value. Why not use errno for all "int" return value across all initialization/finalization functions? And in the caller, you can: + if (ret) { + pr_debug("ACPI PCC probe failed.\n"); + return ret; + } BTW, wasn't it better to return -ENODEV here? Thanks -Lv > > Thanks, > Ashwin > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
